Skip to content

http: preserve socket errors as response error causes - #66061

Open
XadillaX wants to merge 1 commit into
nodejs:mainfrom
XadillaX:fix-http-response-socket-error
Open

XadillaX wants to merge 1 commit into
nodejs:mainfrom
XadillaX:fix-http-response-socket-error

Conversation

@XadillaX

@XadillaX XadillaX commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

When a socket error interrupts an HTTP response, the request receives the original error, but the response and its downstream pipeline receive only ECONNRESET: aborted. The underlying cause is lost.

Preserve that original error as cause on the response error, keeping the existing ECONNRESET code, aborted message, and event order. This applies to TLS errors and actual TCP resets alike. Premature closure without an underlying error leaves cause absent.

This came out of investigating #66001. I could not reproduce or identify the root cause of the reported TLS decryption failure on an intact stream, but confirmed the separate loss of error context described above. The original failure remains unresolved; this PR is limited to preserving diagnostic context. The TLS regression test deliberately sends an invalid record after response headers have been received to test error propagation.

Validation on macOS arm64:

  • Release build succeeded.
  • All 721 selected HTTP, HTTPS, TLS, error, and TCP reset tests passed.
  • Regression coverage includes TLS 1.2/1.3, real TCP resets, explicit destruction, closure without an error, cause identity and property attributes, and event order.
  • The TLS regression test fails on unmodified Node v24.21.0 because cause is missing.
  • JavaScript lint, Markdown lint, and git diff --check passed.

Refs: #66001

AI assistance: GPT-6 assisted with investigation, implementation, tests, and this description. The compatibility approach was selected through discussion with the contributor; the validation above was run by the assistant.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originating in Node.js core. http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Sep 16, 2026
Keep the original socket error as the cause of ECONNRESET errors emitted
when an HTTP response closes before completion. Preserve the existing
aborted message, error code, and event order. Leave cause absent when
there is no underlying error.

Allow ConnResetException to accept Error options, document the behavior,
and cover TLS record errors, TCP resets, explicit destruction, and
premature closure without a socket error.

This follows investigation of nodejs#66001 and addresses lost error context.
The original TLS decryption failure remains unresolved.

Refs: nodejs#66001
Signed-off-by: XadillaX <[email protected]>
@XadillaX
XadillaX force-pushed the fix-http-response-socket-error branch from dceab63 to cd91d44 Compare September 16, 2026 12:21
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.26%. Comparing base (565f69f) to head (cd91d44).
⚠️ Report is 81 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66061      +/-   ##
==========================================
+ Coverage   89.99%   90.26%   +0.26%     
==========================================
  Files         784      789       +5     
  Lines      268410   271477    +3067     
  Branches    51124    51811     +687     
==========================================
+ Hits       241562   245048    +3486     
+ Misses      17385    16919     -466     
- Partials     9463     9510      +47     
Files with missing lines Coverage Δ
lib/_http_client.js 97.66% <100.00%> (+0.02%) ⬆️
lib/internal/errors.js 98.81% <100.00%> (-0.01%) ⬇️

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

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

Labels

errors Issues and PRs related to JavaScript errors originating in Node.js core. http Issues and 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.

2 participants