http: add req.signal to IncomingMessage by akshatsrivastava11 · Pull Request #62541 · nodejs/node

akshatsrivastava11 · 2026-04-01T17:51:54Z

Summary

Adds a lazy signal getter to IncomingMessage that returns an
AbortSignal which aborts when the request is closed or aborted.
This mirrors the Web Fetch API's Request.signal and Deno's
request.signal.

Motivation

Currently, cancelling async work (DB queries, fetch calls) when a
client disconnects requires manual boilerplate in every request handler:

// before
server.on('request', async (req, res) => {
  const ac = new AbortController();
  req.on('close', () => ac.abort());
  const data = await fetch('https://slow-api.com', { signal: ac.signal });
  res.end(JSON.stringify(data));
});

With this change:

// after
server.on('request', async (req, res) => {
  const data = await fetch('https://slow-api.com', { signal: req.signal });
  res.end(JSON.stringify(data));
});

Design

Lazy initialization — AbortController is only created when
req.signal is first accessed. Zero overhead for handlers that
do not use it.

Single listener — listens on this.once('close') and
this.once('abort') rather than socket.once('close') directly,
since the request stream's close event fires in all teardown paths:

socket close propagates through _destroy() to req close
socketless req.destroy() fires req close directly
client abort fires req abort directly

Race condition — if .signal is accessed after req.destroy()
has already fired, this.destroyed is checked and the signal is
aborted immediately rather than registering a listener that would
never fire.

configurable: true — allows frameworks (Express, Fastify, Koa)
to override the property on their own subclasses.

Changes

lib/_http_incoming.js — adds signal getter to IncomingMessage
test/parallel/test-http-request-signal.js — adds tests

Tests

req.signal is an AbortSignal and not aborted initially
signal aborts when 'abort' event fires
signal aborts when 'close' event fires (client disconnect)
signal is pre-aborted if accessed after req.destroy() (race condition)
multiple accesses return the same signal instance (lazy init)

Fixes: #62481

nodejs-github-bot · 2026-04-01T17:52:00Z

Review requested:

@nodejs/http
@nodejs/net

akshatsrivastava11 · 2026-04-01T17:54:50Z

@mcollina
could you please let me know if this is along the lines of what you had in mind, or if I should change anything ?

mcollina

Can you add a test how this would be used in http.request() response?

codecov · 2026-04-01T19:58:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (0dfdec9) to head (18f6103).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62541      +/-   ##
==========================================
- Coverage   89.71%   89.66%   -0.06%     
==========================================
  Files         695      693       -2     
  Lines      214154   213103    -1051     
  Branches    41009    40861     -148     
==========================================
- Hits       192132   191082    -1050     
- Misses      14075    14142      +67     
+ Partials     7947     7879      -68

Files with missing lines	Coverage Δ
lib/_http_incoming.js	`99.38% <100.00%> (+0.03%)`	⬆️

... and 117 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

akshatsrivastava11 · 2026-04-02T03:59:41Z

@mcollina I've added a test for the http.request() response case in the latest commit. It creates a server that sends partial data and keeps the connection open, then destroys the client request mid-response and verifies that res.signal aborts. Let me know if that's what you had in mind or if anything needs adjusting.

slagiewka

Great to see it happening! WHATWG Request wrappers will be much easier to write 👍

slagiewka · 2026-04-02T07:57:15Z

lib/_http_incoming.js

+    if (this[kAbortController] === undefined) {
+      const ac = new AbortController();
+      this[kAbortController] = ac;
+      if (this.destroyed) {
+        ac.abort();


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();

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.

slagiewka · 2026-04-02T08:09:58Z

lib/_http_incoming.js

+        this.once('close', function() {
+          ac.abort();
+        });
+        this.once('abort', function() {


question: I'm not well versed in node.js internals, but when would this happen?
There's the deprecated "aborted" event which I assume the stdlib does not need to care about if using 'close'.
And AbortSignal does indeed use "abort" but I doubt that emitting abort on IncomingMessage is something that's expected. There's something similar on the http.ClientRequest but that's the client side aborting the outgoing request.

+1, we should check & test to confirm, but my expectation would be that this is never emitted for incoming messages, that's certainly what our documentation says.

The 'abort' listener was added based on @mcollina's suggestion. If it turns out this is dead code or not needed for IncomingMessage, I'm happy to remove it ,or maybe I misinterpreted the suggestion. @mcollina could you clarify the intended use case for this?

metcoder95 · 2026-04-02T08:19:29Z

lib/_http_incoming.js

  // 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;


nit: set to null instead to reflect object

Thanks for the suggestion, updated in the latest commit

pimterry · 2026-04-02T09:44:30Z

lib/_http_incoming.js

+        this.once('close', function() {
+          ac.abort();
+        });
+        this.once('abort', function() {


+1, we should check & test to confirm, but my expectation would be that this is never emitted for incoming messages, that's certainly what our documentation says.

pimterry · 2026-04-02T09:53:19Z

test/parallel/test-http-request-signal.js

+    clientReq.on('error', () => {});
+    clientReq.end();
+  }));
+}


I think the most important case here isn't covered: server receives a request & doesn't end the response (stays pending) => client aborts the request while it's incomplete (so the connection closes) => server should see req.signal.aborted == true. That's the primary end goal, allowing us to easily detect on the server side when the client cancelled the requests.

The two full-flow tests here (1 and 5) only cover serverRes.destroy() => serverReq.signal.aborted and clientReq.destroy() => clientRes.signal.aborted.

Looks like it should work, but we need to test to make sure we actually cover that integration.

Thanks,I have added the test in the latest commit

…borted on client disconnect

http: add req.signal to IncomingMessage

e265c6d

nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Apr 1, 2026

ronag approved these changes Apr 1, 2026

View reviewed changes

mcollina requested changes Apr 1, 2026

View reviewed changes

test: add test for res.signal on client side http.request() response

18f6103

slagiewka reviewed Apr 2, 2026

View reviewed changes

mcollina requested a review from jasnell April 2, 2026 08:14

metcoder95 reviewed Apr 2, 2026

View reviewed changes

http: initialize kAbortController to null instead of undefined

b9a1333

pimterry reviewed Apr 2, 2026

View reviewed changes

http: remove abort listener and add test for server-side req.signal.a…

bdf63e1

…borted on client disconnect

Uh oh!

Conversation

akshatsrivastava11 commented Apr 1, 2026

Summary

Motivation

Design

Changes

Tests

Uh oh!

nodejs-github-bot commented Apr 1, 2026

Uh oh!

akshatsrivastava11 commented Apr 1, 2026

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

akshatsrivastava11 commented Apr 2, 2026

Uh oh!

slagiewka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov bot commented Apr 1, 2026 •

edited

Loading