Skip to content

Commit

Permalink
http: add support for abortsignal to http.request
Browse files Browse the repository at this point in the history
PR-URL: #36048
Backport-PR-URL: #38386
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
benjamingr authored and targos committed Apr 30, 2021
1 parent 6e734a8 commit 524b713
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 5 deletions.
9 changes: 9 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -2326,6 +2326,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 @@ -2393,6 +2396,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 @@ -2580,6 +2585,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 @@ -95,7 +95,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 @@ -704,6 +704,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');
this.code = 'ABORT_ERR';
this.name = 'AbortError';
}
}
module.exports = {
addCodeToName, // Exported for NghttpError
codes,
Expand All @@ -718,6 +728,7 @@ module.exports = {
uvException,
uvExceptionWithHostPort,
SystemError,
AbortError,
// This is exported only to facilitate testing.
E,
kNoOverride,
Expand Down
27 changes: 25 additions & 2 deletions test/parallel/test-http-client-abort-destroy.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --experimental-abortcontroller
'use strict';
const common = require('../common');
const http = require('http');
Expand Down Expand Up @@ -52,8 +53,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 +69,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);
}));
}

0 comments on commit 524b713

Please sign in to comment.