fix(dev): backstop uncaughtException for socket errors pipe() misses

[james-elicx]

Summary

Follow-up to #911. The connection-level guard installed there fixes the direct res.socket case but doesn't cover Node's pipe() re-emission path. When a Readable source errors and the destination has no 'error' listener, pipe() emits the source's error onto the destination — and the destination isn't always the inbound connection socket.

Real call sites in vinext that hit this:

• fromWeb(fetch().body).pipe(res) in proxyExternalRewriteNode — source is the upstream fetch body stream, not an inbound socket.
• Streaming surfaces inside @vitejs/plugin-rsc that own their own pipe topology.
• Outbound sockets created by fetch() from middleware — these never fire 'connection' on server.httpServer, so they sit entirely outside the existing guard.

Stack trace from the field (vp dlx vinext@54c91f1 dev, after a successful GET /stage/oliver):

Error: read ECONNRESET
    at TCP.onStreamRead (node:internal/stream_base_commons:216:20)
Emitted 'error' event on Socket instance at:
    at Socket.onerror (node:internal/streams/readable:1035:14)
    at Socket.emit (node:events:509:28)
    ...

Socket.onerror at internal/streams/readable:1035 is pipe()'s internal error-propagation function — same crash shape that #911 set out to fix, different installation site.

Approach

Add a process-level uncaughtException handler scoped to the dev server. It drops only peer-disconnect codes (ECONNRESET, EPIPE, ECONNABORTED) and re-throws everything else on nextTick, which preserves Node's default crash semantics for real bugs. The handler is removed on httpServer 'close' so it doesn't leak across restarts.

Why a process-level backstop:

• It's the only place that catches outbound socket errors from fetch() and pipe re-emissions to non-socket destinations.
• Filtering by error code keeps it safe — non-disconnect errors still surface exactly as Node would have surfaced them.
• Dev-only: lives inside configureServer, never installed in build/prod.

Refs

• Refs Dev server crashes with Unhandled 'error' event (ECONNRESET) when editing files #905 (original issue — re-occurring after fix(dev): swallow socket errors so peer disconnects don't crash dev server #911 partially closed it)
• Builds on fix(dev): swallow socket errors so peer disconnects don't crash dev server #911

Test plan

• vp check passes (note: 12 pre-existing typecheck errors about missing peer deps are unchanged)
• Manual repro from Dev server crashes with Unhandled 'error' event (ECONNRESET) when editing files #905 with proxyExternalRewriteNode in play: dev server no longer crashes after browser disconnect mid-stream
• Genuine errors thrown from request handlers still crash dev server with full stack (handler re-throws on nextTick)
• httpServer.close() followed by another listen() doesn't double-register the handler

🤖 Generated with Claude Code

[james-elicx]

/bigbonk review

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@913

commit: 95cd980

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original

[ask-bonk]

Reviewer notes

I confirmed the diagnosis of the original crash (pipe re-emission from non-socket destinations and outbound fetch() sockets) — the connection-level guard from #911 genuinely doesn't cover those paths, and a process-level uncaughtException handler is a reasonable backstop. Next.js installs one in dev for similar reasons (packages/next/src/server/lib/router-server.ts ~line 809).

However, the re-throw path for non-disconnect errors does not behave the way the comment claims, and that's a meaningful behavioral regression for genuine errors.

Blocker: re-throw on nextTick creates an infinite loop, doesn't crash

packages/vinext/src/index.ts:2020-2028:

const onUncaught = (err: Error) => {
  if (isPeerDisconnect(err)) return;
  process.nextTick(() => {
    throw err;
  });
};
process.on("uncaughtException", onUncaught);

The comment says this restores Node's default crash behavior — "same shape as not having installed a handler at all." That's not what happens. Node's default crash only fires when there are no uncaughtException listeners. Here the listener is still installed when the nextTick callback runs, so the re-thrown error is caught by the same handler again, which schedules another nextTick throw, which is caught again, forever.

Reproduced locally with the exact pattern:

$ node -e '
process.on(\"uncaughtException\", (err) => {
  console.log(\"caught:\", err.message);
  if (err.code === \"ECONNRESET\") return;
  process.nextTick(() => { throw err; });
});
process.nextTick(() => { throw new Error(\"genuine bug\"); });
setTimeout(() => { console.log(\"still alive\"); process.exit(0); }, 100);
'
caught: genuine bug
caught: genuine bug
caught: genuine bug
... (repeats indefinitely)
still alive

Net effect for vinext dev users: a real programming error that previously would crash the dev server with a stack trace now gets swallowed silently and the event loop spins on it. That is the opposite of the stated intent ("keep real bugs surfacing").

This also means the unchecked test-plan item "Genuine errors thrown from request handlers still crash dev server with full stack" will not pass as written.

Fix options

Pick whichever fits the style of the file; any of these restores the intended semantics:

1. Synchronous re-throw inside the handler (simplest):

const onUncaught = (err: Error) => {
  if (isPeerDisconnect(err)) return;
  throw err; // sync throw inside the handler aborts the process with code 1
};

I verified Node treats a synchronous throw inside an uncaughtException listener as fatal (exit code 7 / 1 depending on version), printing the stack — same observable as no handler.

2. Remove the listener, then re-throw asynchronously:

const onUncaught = (err: Error) => {
  if (isPeerDisconnect(err)) return;
  process.removeListener(\"uncaughtException\", onUncaught);
  process.nextTick(() => { throw err; });
};

3. Log + process.exit(1) — matches what Next.js's setup-exception-listeners.ts does. Loses the default stack-trace formatting unless you console.error(err) first.

I'd lean toward (1) for minimum diff and matching Node's default-crash output most closely.

Minor: leak in middleware mode

server.httpServer?.once(\"close\", ...) is a no-op when server.httpServer is null (Vite middleware mode — vinext embedded in a host server). The uncaughtException listener is never cleaned up in that case. Not a hot path today (no test uses middlewareMode), but worth tying the cleanup to something that always exists, e.g. also hook server.watcher.close or a buildEnd hook, or skip installation entirely when httpServer is null.

Nit: also catch unhandledRejection?

The two real crash sources cited (pipe re-emission, outbound fetch() socket errors from middleware) can both reach the process as unhandledRejection rather than uncaughtException depending on which async surface throws. If the goal is a true backstop for peer-disconnect errors in dev, mirroring the same filter on unhandledRejection would close that gap. Optional — fine to leave for a follow-up if you want to keep this PR tight.

Once the re-throw is fixed

Approach LGTM otherwise. Filter is correctly scoped to dev (configureServer only), correctly scoped to peer-disconnect codes, and the cleanup-on-close pattern is reasonable for the httpServer != null case.

[ask-bonk]

Review posted as Changes Requested on PR #913.

Summary of findings:

1. Blocker (verified by reproduction): The process.nextTick(() => { throw err }) re-throw pattern at packages/vinext/src/index.ts:2025-2027 creates an infinite loop for genuine errors, not a crash. Because the listener is still installed when the nextTick callback fires, the re-thrown error is caught by the same handler again. Node's default crash only fires when there are zero uncaughtException listeners — so the comment's claim "same shape as not having installed a handler at all" is incorrect. Net effect: real programming errors get silently swallowed in dev, opposite of intent. Suggested three fix variants (sync throw inside handler is simplest).

2. Minor: server.httpServer?.once("close", ...) is a no-op in Vite middlewareMode (no httpServer), so the listener leaks across the lifetime of the embedding process.

3. Nit: unhandledRejection could reach the process from the same async surfaces; consider mirroring the filter there too as a follow-up.

Approach itself is sound — process-level backstop is the right shape, filter codes are correct, scope to dev-only via configureServer is right.

github run

[james-elicx]

Thanks for the catch — all three addressed in 95cd980.

Blocker (1): Confirmed the loop locally — your repro reproduces verbatim. Switched to a synchronous throw inside the handler. Verified Node treats it as fatal:

$ node -e 'process.on("uncaughtException", (err) => { if (err.code === "ECONNRESET") return; throw err; }); process.nextTick(() => { throw new Error("genuine bug"); });'
[eval]:1
... uncaughtException listener ... throw err;
                                   ^
Error: genuine bug
    at [eval]:1:...
Node.js v24.15.0

Stack to stderr, non-zero exit — same observable as no listener.

Minor (2): Skipped installation entirely when httpServer is null. Wrapped the whole block in if (server.httpServer) so neither the listeners nor the close hook are registered in middleware mode. Embedding host stays in charge of process handlers, no leak.

Nit (3): Folded in — same isPeerDisconnect filter mirrored onto unhandledRejection with the same sync-throw shape, removed alongside the uncaughtException handler on close.

[james-elicx]

/bigbonk re-review for issues

[ask-bonk]

Re-review of 95cd980

All three points from the previous review are addressed correctly. Verified locally.

Blocker (1) — sync throw: ✅ fixed

packages/vinext/src/index.ts:2030-2033 and :2034-2037 use a synchronous throw inside the listener instead of process.nextTick(() => { throw err }). Reproduced both shapes:

$ node -e 'process.on("uncaughtException", (err) => { if (err.code === "ECONNRESET") return; throw err; });
           process.nextTick(() => { throw new Error("genuine bug"); });
           setTimeout(() => { console.log("STILL ALIVE"); }, 200);'
[eval]:4
  throw err;
  ^
Error: genuine bug
    at [eval]:6:32
Node.js v24.15.0
# exit=7

No loop, stack to stderr, non-zero exit. ECONNRESET still passes through silently as intended (verified separately).

Minor (2) — middleware-mode leak: ✅ fixed

Wrapping the entire block in if (server.httpServer) (line 2020) is the right call. Embedding host keeps ownership of process handlers; nothing leaks. Consistent with the existing server.httpServer?.on("connection", ...) pattern just above.

Nit (3) — unhandledRejection: ✅ folded in

onUnhandledRejection mirrors the same filter and sync-throw shape (:2034-2037) and is removed alongside onUncaught on close. Verified throw reason inside an unhandledRejection listener also crashes Node fatally (exit 1, stack printed), and the ECONNRESET filter still drops peer disconnects from rejected promises.

One subtle property worth noting: rejected non-Error values (Promise.reject("string"), Promise.reject(null)) flow through fine — (reason as {code?: string} | null)?.code is undefined for primitives and null, so isPeerDisconnect returns false and the throw still surfaces them. Good.

Other observations (non-blocking)

• Listener accumulation on Vite restart: if Vite reloads the config and re-runs configureServer while the previous httpServer hasn't yet fired 'close', listeners briefly stack. Node warns at 10. Realistically not hit in normal dev, and the close hook does eventually clean up. Fine to leave.
• No automated test: process-level error handlers are genuinely awkward to test — would require a child-process harness. The manual repro in the PR description is the right level of validation for a dev-only safety net.
• Wrapped errors: new Error("wrapped", { cause: econnreset }) won't be filtered (top-level code is undefined). Matches the existing socket-level guard's behavior and is the conservative choice — only drop errors that directly look like peer disconnect.

LGTM.

[ask-bonk]

Review posted as APPROVED on PR #913.

Summary of findings:

All three points from my previous review were addressed correctly in commit 95cd980:

1. Blocker fixed: Switched from process.nextTick(() => { throw err }) to a synchronous throw err inside the listener. Verified locally — Node treats it as fatal (exit 7, stack to stderr), no infinite loop. Real bugs surface as intended.

2. Middleware-mode leak fixed: Entire block now wrapped in if (server.httpServer), so listeners aren't installed when the embedding host owns the process.

3. unhandledRejection mirror added: Same filter, same sync-throw shape, cleaned up alongside uncaughtException on close. Verified the throw-inside-listener pattern is fatal there too, and that primitives/null reasons flow through correctly without false positives.

Flagged three non-blocking observations (listener accumulation on Vite config reload, lack of automated test, wrapped-error chains not filtered) but none are worth holding the PR for.

github run