fix(http): handle socket errors to prevent EPIPE process crash#10662
fix(http): handle socket errors to prevent EPIPE process crash#10662vitingr wants to merge 3 commits intoaxios:v1.xfrom
Conversation
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- There is a concrete behavior risk in
lib/adapters/http.js: the socket error listener closes over the first request, so on keep-alive socket reuse an error may be routed to a stale request instead of the current one. - This is a medium-severity, medium-confidence issue (5/10, 6/10) with potential user-facing impact in error propagation, which makes merge risk non-trivial but not clearly blocking.
- Pay close attention to
lib/adapters/http.js- socket error handling on reused connections may target the wrong request context.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/adapters/http.js">
<violation number="1" location="lib/adapters/http.js:890">
P2: Socket error listener is attached only once per socket but captures the first request in its closure, so errors on reused keep‑alive sockets can be forwarded to a stale request instead of the active one, preventing proper rejection and retaining the old request.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
3981468 to
8d7a518
Compare
|
@cubic-dev-ai The stale closure issue has been addressed. |
@vitingr This updated approach effectively solves the stale closure problem. By moving the request reference to a property on the socket ( Using Symbols is the right choice here to avoid name collisions on the socket object, which is especially important since these sockets are often managed by a global or shared One detail to note: since You can find more about how I track these types of patterns in my documentation or view your team's saved repository learnings. |
|
@cubic-dev-ai please re-review the entire PR please. |
@vitingr I have started the AI code review. It will take a few minutes to complete. |
…de-process-with-epipe
…de-process-with-epipe
|
Thanks closing in favour of #10576 |
Problem
When uploading a large body, the server can close the connection before
the client finishes writing. Node.js emits an
errorevent on the socketwith code
EPIPE. Since Axios had noerrorlistener on the socket, thisbecame an
uncaughtExceptionand crashed the process on Linux/macOS.Fixes #10558
Root cause
In
lib/adapters/http.js, thehandleRequestSocketcallback set upkeep-alive but never attached a socket-level error handler.
Solution
Attach a single
errorlistener per socket using a Symbol guard to avoidleaking listeners on keep-alive connections that are reused across requests.
The error is forwarded via
req.destroy(err), which triggers the existinghandleRequestErrorhandler, so the promise rejects cleanly instead ofcrashing the process.
Testing
Notes
The
fetchadapter is not affected (handled internally by the Fetch API)Windows does not exhibit the crash due to
socketErrorListenerinNode's
_http_client, but the fix is still necessary for Linux/macOSproduction environments
Before
After
I've used this .js script to test the issue in windows environment:
Summary by cubic
Prevents Node processes from crashing on Linux/macOS by handling socket error events (e.g., EPIPE) in the Node
httpadapter. Errors are now forwarded to the active keep‑alive request and cleanly reject inaxiosinstead of triggeringuncaughtException.Description
Summary of changes
errorlistener viaSOCKET_ERROR_HANDLEDin the Nodehttpadapter.SOCKET_CURRENT_REQUESTand forward errors usingcurrentReq.destroy(err).close.Reasoning
EPIPE/ECONNRESETon keep‑alive sockets causeduncaughtException. Ensures the right request is rejected.Additional context
lib/adapters/http.js; no API changes.fetchadapter is unaffected.Testing
EPIPE) without process crash.Written for commit 85b4662. Summary will update on new commits.