Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: add support for abortsignal to http.request #36048

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -2336,6 +2336,9 @@ This can be overridden for servers and client requests by passing the
<!-- YAML
added: v0.3.6
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36048
description: It is possible to abort a request with an AbortSignal.
- version:
- v13.8.0
- v12.15.0
Expand Down Expand Up @@ -2403,6 +2406,8 @@ changes:
or `port` is specified, those specify a TCP Socket).
* `timeout` {number}: A number specifying the socket timeout in milliseconds.
This will set the timeout before the socket is connected.
* `signal` {AbortSignal}: An AbortSignal that may be used to abort an ongoing
request.
* `callback` {Function}
* Returns: {http.ClientRequest}

Expand Down Expand Up @@ -2596,6 +2601,10 @@ events will be emitted in the following order:
Setting the `timeout` option or using the `setTimeout()` function will
not abort the request or do anything besides add a `'timeout'` event.

Passing an `AbortSignal` and then calling `abort` on the corresponding
`AbortController` will behave the same way as calling `.destroy()` on the
request itself.

## `http.validateHeaderName(name)`
<!-- YAML
added: v14.3.0
Expand Down
2 changes: 1 addition & 1 deletion lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ rules:
- selector: "ThrowStatement > CallExpression[callee.name=/Error$/]"
message: "Use new keyword when throwing an Error."
# Config specific to lib
- selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])"
- selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError)$/])"
message: "Use an error exported by the internal/errors module."
- selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']"
message: "Please use `require('internal/errors').hideStackFrames()` instead."
Expand Down
16 changes: 14 additions & 2 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,18 @@ const { Buffer } = require('buffer');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { URL, urlToOptions, searchParamsSymbol } = require('internal/url');
const { kOutHeaders, kNeedDrain } = require('internal/http');
const { connResetException, codes } = require('internal/errors');
const { AbortError, connResetException, codes } = require('internal/errors');
const {
ERR_HTTP_HEADERS_SENT,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_HTTP_TOKEN,
ERR_INVALID_PROTOCOL,
ERR_UNESCAPED_CHARACTERS
} = codes;
const { validateInteger } = require('internal/validators');
const {
validateInteger,
validateAbortSignal,
} = require('internal/validators');
const { getTimerDuration } = require('internal/timers');
const {
DTRACE_HTTP_CLIENT_REQUEST,
Expand Down Expand Up @@ -169,6 +172,15 @@ function ClientRequest(input, options, cb) {
if (options.timeout !== undefined)
this.timeout = getTimerDuration(options.timeout, 'timeout');

const signal = options.signal;
if (signal) {
validateAbortSignal(signal, 'options.signal');
const listener = (e) => this.destroy(new AbortError());
signal.addEventListener('abort', listener);
this.once('close', () => {
signal.removeEventListener('abort', listener);
});
}
let method = options.method;
const methodIsString = (typeof method === 'string');
if (method !== null && method !== undefined && !methodIsString) {
Expand Down
11 changes: 11 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,16 @@ const fatalExceptionStackEnhancers = {
}
};

// Node uses an AbortError that isn't exactly the same as the DOMException
// to make usage of the error in userland and readable-stream easier.
// It is a regular error with `.code` and `.name`.
class AbortError extends Error {
constructor() {
super('The operation was aborted');
ronag marked this conversation as resolved.
Show resolved Hide resolved
this.code = 'ABORT_ERR';
this.name = 'AbortError';
}
}
module.exports = {
addCodeToName, // Exported for NghttpError
codes,
Expand All @@ -741,6 +751,7 @@ module.exports = {
uvException,
uvExceptionWithHostPort,
SystemError,
AbortError,
// This is exported only to facilitate testing.
E,
kNoOverride,
Expand Down
26 changes: 24 additions & 2 deletions test/parallel/test-http-client-abort-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ const assert = require('assert');
{
// destroy

const server = http.createServer(common.mustNotCall((req, res) => {
}));
const server = http.createServer(common.mustNotCall());

server.listen(0, common.mustCall(() => {
const options = { port: server.address().port };
Expand All @@ -69,3 +68,26 @@ const assert = require('assert');
assert.strictEqual(req.destroyed, true);
}));
}


{
// Destroy with AbortSignal

const server = http.createServer(common.mustNotCall());
const controller = new AbortController();

server.listen(0, common.mustCall(() => {
const options = { port: server.address().port, signal: controller.signal };
const req = http.get(options, common.mustNotCall());
req.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ABORT_ERR');
assert.strictEqual(err.name, 'AbortError');
server.close();
}));
assert.strictEqual(req.aborted, false);
assert.strictEqual(req.destroyed, false);
controller.abort();
assert.strictEqual(req.aborted, false);
assert.strictEqual(req.destroyed, true);
}));
}