Skip to content

http: avoid aborting IncomingMessage signal on normal close#64392

Open
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:httpfix
Open

http: avoid aborting IncomingMessage signal on normal close#64392
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:httpfix

Conversation

@Archkon

@Archkon Archkon commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes: #64390

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 9, 2026
IncomingMessage 'close' is emitted when request handling completes, not
only when the underlying connection is closed. Using that event to abort
IncomingMessage.signal makes the signal abort after a request body is
read normally.

Track the underlying socket close instead, and detach the listener when
the request or response completes normally. This keeps the signal from
being aborted by normal stream completion or later keep-alive socket
closure, while still aborting it when the connection closes during the
active request lifecycle.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.18519% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.24%. Comparing base (8fec65d) to head (5f09def).

Files with missing lines Patch % Lines
lib/_http_incoming.js 84.31% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64392      +/-   ##
==========================================
- Coverage   90.25%   90.24%   -0.02%     
==========================================
  Files         741      741              
  Lines      241207   241257      +50     
  Branches    45430    45440      +10     
==========================================
+ Hits       217698   217714      +16     
- Misses      15084    15089       +5     
- Partials     8425     8454      +29     
Files with missing lines Coverage Δ
lib/_http_client.js 97.59% <100.00%> (+<0.01%) ⬆️
lib/_http_server.js 96.64% <100.00%> (+0.13%) ⬆️
lib/_http_incoming.js 97.93% <84.31%> (-1.45%) ⬇️

... and 32 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

req.signal.aborted becomes true after request body parse / stream close in Node 24.18.0

2 participants