Keep the server alive when a response write hits a dead socket#4470
Keep the server alive when a response write hits a dead socket#4470yashranaway wants to merge 1 commit into
Conversation
A client that disconnects while an error response is being written crashes the whole Node server with an unhandled EPIPE, disconnecting every other client and abandoning in-flight provider work. Upgrade sockets are the worst case: once a connection upgrades for websocket RPC, Node's http server detaches its own socket error handling, so an auth rejection written to a vanished client emits an error event with no listener at all. Arm every response and upgrade socket with an error listener when the Node server is created. The failed request is already interrupted through its close event, so the write failure only needs to be observed instead of taking down the process. Fixes pingdotgg#4410
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Approved Defensive bug fix that adds error listeners to prevent uncaught socket write errors from crashing the server process. The change is minimal, well-tested, and doesn't alter business logic - just catches errors that would otherwise terminate the server. You can customize Macroscope's approvability policy. Learn more. |
What Changed
The Node HTTP server is created through a small guard that attaches an "error" listener to every server response and every upgrade socket. The listener only observes the failure; a new test reproduces the incident shape and proves the process survives and keeps serving.
Why
A client that disconnects while an error response is being written crashes the whole server with an unhandled EPIPE, disconnecting every other client and abandoning in-flight provider work.
Upgrade sockets are the worst case and match the reported incident: once a connection upgrades for websocket RPC, Node's http server detaches its own socket error handling, so an auth rejection written to a vanished client emits an "error" event with no listener anywhere, which escalates to an uncaught exception and terminates the process. Plain response streams have no default error listener either.
The failed request needs no recovery logic: its fiber is already interrupted through the response "close" event. The write failure only needs a listener so it stays contained to the request that died.
Fixes #4410
Testing
vp test run apps/server/src/httpResponseErrorGuard.test.tspasses (2/2): an EPIPE on an upgrade socket is contained and the server still answers a follow-up request; normal traffic is untouched and every response carries an error listenervp test run apps/server/src/server.test.tspasses (112/112)pnpm typecheckin apps/server cleanvp linton changed files cleanChecklist
Note
Medium Risk
Touches core Node HTTP server bootstrap; behavior is limited to swallowing/containing socket write errors without changing request handling logic.
Overview
Prevents the Node HTTP server from exiting when a client disconnects mid-write (e.g. EPIPE on websocket upgrade / auth rejection paths).
Adds
guardHttpResponseWriteErrors, which attacheserrorlisteners on every response and upgrade socket so unhandled write failures are observed instead of becoming process-fatal uncaught exceptions. The main server now wrapsNodeHttp.createServer()with this guard (Bun path unchanged). New tests assert an upgrade-socket EPIPE is contained and normal requests still succeed.Reviewed by Cursor Bugbot for commit 5970c0f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Prevent uncaught exceptions when writing to dead sockets in the Node HTTP server
Introduces
guardHttpResponseWriteErrorsin httpResponseErrorGuard.ts, which attacheserrorlisteners to HTTP response objects and upgrade sockets, routing write errors (e.g.EPIPE) to an optional callback instead of letting them propagate as uncaught exceptions.HttpServerLivein server.ts wrapsNodeHttp.createServer()with this guard so all response and upgrade socket errors are captured automatically.Macroscope summarized 5970c0f.