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
Backport-PR-URL: #38386
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
MadaraUchiha authored and targos committed Apr 30, 2021
1 parent 524b713 commit 336fb18
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 6 deletions.
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: v10.10.0
pr-url: https://github.com/nodejs/node/pull/22466
description: HTTP/2 is now Stable. Previously, it had been Experimental.
Expand Down Expand Up @@ -819,6 +822,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 @@ -855,6 +860,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
27 changes: 22 additions & 5 deletions lib/internal/http2/core.js
Expand Up @@ -94,12 +94,15 @@ const {
ERR_OUT_OF_RANGE,
ERR_SOCKET_CLOSED
},
hideStackFrames
hideStackFrames,
AbortError
} = require('internal/errors');
const { validateNumber,
validateString,
validateUint32,
isUint32,
const {
isUint32,
validateNumber,
validateString,
validateUint32,
validateAbortSignal,
} = require('internal/validators');
const fsPromisesInternal = require('internal/fs/promises');
const { utcDate } = require('internal/http');
Expand Down Expand Up @@ -1666,6 +1669,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 = requestOnConnect.bind(stream, headersList, options);
if (this.connecting) {
if (this[kPendingRequestCalls] !== null) {
Expand Down
76 changes: 75 additions & 1 deletion test/parallel/test-http2-client-destroy.js
@@ -1,4 +1,4 @@
// Flags: --expose-internals
// Flags: --expose-internals --experimental-abortcontroller

'use strict';

Expand All @@ -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 @@ -165,3 +166,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 336fb18

Please sign in to comment.