Skip to content

Commit

Permalink
http2: add support for AbortSignal to http2Session.request
Browse files Browse the repository at this point in the history
- Add support
- Add test
- Docs once PR is up

PR-URL: #36070
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
MadaraUchiha authored and codebytere committed Nov 22, 2020
1 parent 79b2ba6 commit 48bf59b
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 1 deletion.
9 changes: 9 additions & 0 deletions doc/api/http2.md
Expand Up @@ -2,6 +2,9 @@
<!-- YAML
added: v8.4.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36070
description: It is possible to abort a request with an AbortSignal.
- version: v15.0.0
pr-url: https://github.com/nodejs/node/pull/34664
description: Requests with the `host` header (with or without
Expand Down Expand Up @@ -846,6 +849,8 @@ added: v8.4.0
and `256` (inclusive).
* `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
`'wantTrailers'` event after the final `DATA` frame has been sent.
* `signal` {AbortSignal} An AbortSignal that may be used to abort an ongoing
request.

* Returns: {ClientHttp2Stream}

Expand Down Expand Up @@ -882,6 +887,10 @@ close when the final `DATA` frame is transmitted. User code must call either
`http2stream.sendTrailers()` or `http2stream.close()` to close the
`Http2Stream`.

When `options.signal` is set with an `AbortSignal` and then `abort` on the
corresponding `AbortController` is called, the request will emit an `'error'`
event with an `AbortError` error.

The `:method` and `:path` pseudo-headers are not specified within `headers`,
they respectively default to:

Expand Down
18 changes: 17 additions & 1 deletion lib/internal/http2/core.js
Expand Up @@ -109,7 +109,8 @@ const {
ERR_OUT_OF_RANGE,
ERR_SOCKET_CLOSED
},
hideStackFrames
hideStackFrames,
AbortError
} = require('internal/errors');
const {
isUint32,
Expand All @@ -118,6 +119,7 @@ const {
validateNumber,
validateString,
validateUint32,
validateAbortSignal,
} = require('internal/validators');
const fsPromisesInternal = require('internal/fs/promises');
const { utcDate } = require('internal/http');
Expand Down Expand Up @@ -1721,6 +1723,20 @@ class ClientHttp2Session extends Http2Session {
if (options.waitForTrailers)
stream[kState].flags |= STREAM_FLAGS_HAS_TRAILERS;

const { signal } = options;
if (signal) {
validateAbortSignal(signal, 'options.signal');
const aborter = () => stream.destroy(new AbortError());
if (signal.aborted) {
aborter();
} else {
signal.addEventListener('abort', aborter);
stream.once('close', () => {
signal.removeEventListener('abort', aborter);
});
}
}

const onConnect = FunctionPrototypeBind(requestOnConnect,
stream, headersList, options);
if (this.connecting) {
Expand Down
74 changes: 74 additions & 0 deletions test/parallel/test-http2-client-destroy.js
Expand Up @@ -8,6 +8,7 @@ if (!common.hasCrypto)
const assert = require('assert');
const h2 = require('http2');
const { kSocket } = require('internal/http2/util');
const { kEvents } = require('internal/event_target');
const Countdown = require('../common/countdown');

{
Expand Down Expand Up @@ -167,3 +168,76 @@ const Countdown = require('../common/countdown');
req.on('close', common.mustCall(() => server.close()));
}));
}

// Destroy with AbortSignal
{
const server = h2.createServer();
const controller = new AbortController();

server.on('stream', common.mustNotCall());
server.listen(0, common.mustCall(() => {
const client = h2.connect(`http://localhost:${server.address().port}`);
client.on('close', common.mustCall());

const { signal } = controller;
assert.strictEqual(signal[kEvents].get('abort'), undefined);

client.on('error', common.mustCall(() => {
// After underlying stream dies, signal listener detached
assert.strictEqual(signal[kEvents].get('abort'), undefined);
}));

const req = client.request({}, { signal });

req.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ABORT_ERR');
assert.strictEqual(err.name, 'AbortError');
}));
req.on('close', common.mustCall(() => server.close()));

assert.strictEqual(req.aborted, false);
assert.strictEqual(req.destroyed, false);
// Signal listener attached
assert.strictEqual(signal[kEvents].get('abort').size, 1);

controller.abort();

assert.strictEqual(req.aborted, false);
assert.strictEqual(req.destroyed, true);
}));
}
// Pass an already destroyed signal to abort immediately.
{
const server = h2.createServer();
const controller = new AbortController();

server.on('stream', common.mustNotCall());
server.listen(0, common.mustCall(() => {
const client = h2.connect(`http://localhost:${server.address().port}`);
client.on('close', common.mustCall());

const { signal } = controller;
controller.abort();

assert.strictEqual(signal[kEvents].get('abort'), undefined);

client.on('error', common.mustCall(() => {
// After underlying stream dies, signal listener detached
assert.strictEqual(signal[kEvents].get('abort'), undefined);
}));

const req = client.request({}, { signal });
// Signal already aborted, so no event listener attached.
assert.strictEqual(signal[kEvents].get('abort'), undefined);

assert.strictEqual(req.aborted, false);
// Destroyed on same tick as request made
assert.strictEqual(req.destroyed, true);

req.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ABORT_ERR');
assert.strictEqual(err.name, 'AbortError');
}));
req.on('close', common.mustCall(() => server.close()));
}));
}

0 comments on commit 48bf59b

Please sign in to comment.