Skip to content

http: add req.signal to IncomingMessage #62541

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.

Sign up for GitHub

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

Merged
nodejs-github-bot merged 5 commits into from
Apr 4, 2026
+153 −0
Merged

http: add req.signal to IncomingMessage #62541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e265c6d
http: add req.signal to IncomingMessage
akshatsrivastava11 Apr 1, 2026
18f6103
test: add test for res.signal on client side http.request() response
akshatsrivastava11 Apr 2, 2026
b9a1333
http: initialize kAbortController to null instead of undefined
akshatsrivastava11 Apr 2, 2026
bdf63e1
http: remove abort listener and add test for server-side req.signal.a…
akshatsrivastava11 Apr 2, 2026
48d116f
doc: add message.signal to http IncomingMessage documentation
akshatsrivastava11 Apr 3, 2026
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
26 changes: 26 additions & 0 deletions lib/_http_incoming.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ const {

const { Readable, finished } = require('stream');

const { AbortController } = require('internal/abort_controller');

const kHeaders = Symbol('kHeaders');
const kHeadersDistinct = Symbol('kHeadersDistinct');
const kHeadersCount = Symbol('kHeadersCount');
const kTrailers = Symbol('kTrailers');
const kTrailersDistinct = Symbol('kTrailersDistinct');
const kTrailersCount = Symbol('kTrailersCount');
const kAbortController = Symbol('kAbortController');

function readStart(socket) {
if (socket && !socket._paused && socket.readable)
Expand Down Expand Up @@ -90,6 +93,7 @@ function IncomingMessage(socket) {
// Flag for when we decide that this message cannot possibly be
// read by the user, so there's no point continuing to handle it.
this._dumped = false;
this[kAbortController] = undefined;
Copy link
Copy Markdown
Member

@metcoder95 metcoder95 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: set to null instead to reflect object

Copy link
Copy Markdown
Contributor Author

@akshatsrivastava11 akshatsrivastava11 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, updated in the latest commit

}
ObjectSetPrototypeOf(IncomingMessage.prototype, Readable.prototype);
ObjectSetPrototypeOf(IncomingMessage, Readable);
Expand Down Expand Up @@ -184,6 +188,28 @@ ObjectDefineProperty(IncomingMessage.prototype, 'trailersDistinct', {
},
});

ObjectDefineProperty(IncomingMessage.prototype, 'signal', {
__proto__: null,
configurable: true,
get: function() {
if (this[kAbortController] === undefined) {
const ac = new AbortController();
this[kAbortController] = ac;
if (this.destroyed) {
ac.abort();
Copy link
Copy Markdown

@slagiewka slagiewka Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: if a user calls message.signal on an already destroyed resources, does it make sense to go via new AbortController() + abort route?

Would it make sense to short-circuit into creating aborted Signal without the controller via AbortSignal.abort()?

This would of course require the signal to be stored on the message's symbol props instead of the controller (or along with the controller?).

This might be premature optimisation for an edge case if this doesn't happen often, though.

node --allow-natives-syntax bench.mjs
AbortController + abort                       x 321,009 ops/sec (89 runs sampled) min..max=(2.56us...3.34us)
static abort                                  x 355,118 ops/sec (93 runs sampled) min..max=(2.36us...2.84us)

Both are extremely slow, but skipping the controller is slightly faster.

bench source:

import { Suite } from 'bench-node';

const suite = new Suite({ minSamples: 100 });

suite.add('AbortController + abort', () => {
  const controller = new AbortController();
  controller.abort();
});

suite.add('static abort', () => {
  AbortSignal.abort();
});

suite.run();

Copy link
Copy Markdown
Contributor Author

@akshatsrivastava11 akshatsrivastava11 Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AbortSignal.abort() approach is slightly faster for the pre-destroyed case and avoids creating an unnecessary controller. I kept the current approach for simplicity ,but happy to optimize this in a follow-up if that's the preferred direction.

} else {
this.once('close', function() {
ac.abort();
});
this.once('abort', function() {
Comment thread
mcollina marked this conversation as resolved.
Outdated
ac.abort();
});
}
}
return this[kAbortController].signal;
},
});

IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
if (callback)
this.on('timeout', callback);
Expand Down
44 changes: 44 additions & 0 deletions test/parallel/test-http-request-signal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

// Test 1: req.signal is an AbortSignal and aborts on 'close'
{
const server = http.createServer(common.mustCall((req, res) => {
assert.ok(req.signal instanceof AbortSignal);
assert.strictEqual(req.signal.aborted, false);
req.signal.onabort = common.mustCall(() => {
assert.strictEqual(req.signal.aborted, true);
});
res.destroy();
}));
server.listen(0, common.mustCall(() => {
http.get({ port: server.address().port }, () => {}).on('error', () => {
server.close();
});
}));
}

// Test 2: req.signal is aborted if accessed after destroy
{
const req = new http.IncomingMessage(null);
req.destroy();
assert.strictEqual(req.signal.aborted, true);
}

// Test 3: Multiple accesses return the same signal
{
const req = new http.IncomingMessage(null);
assert.strictEqual(req.signal, req.signal);
}

// Test 4: req.signal aborts when 'abort' event is emitted on req
{
const req = new http.IncomingMessage(null);
const signal = req.signal;
assert.strictEqual(signal.aborted, false);
req.emit('abort');
assert.strictEqual(signal.aborted, true);
}
Loading