[PR #913] [MERGED] fix(dev): backstop uncaughtException for socket errors pipe() misses #943

New issue

Closed

opened 2026-05-06 13:10:59 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:10:59 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/913
Author: @james-elicx
Created: 4/26/2026
Status: ✅ Merged
Merged: 4/26/2026
Merged by: @james-elicx

Base: main ← Head: claude/laughing-proskuriakova-02bf3d

📝 Commits (2)

0d6c8b7 fix(dev): backstop uncaughtException for socket disconnects pipe() misses
95cd980 fix: address bonk review on uncaughtException backstop

📊 Changes

1 file changed (+38 additions, -0 deletions)

View changed files

📝 packages/vinext/src/index.ts (+38 -0)

📄 Description

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 #905 (original issue — re-occurring after #911 partially closed it)
Builds on #911

Test plan

vp check passes (note: 12 pre-existing typecheck errors about missing peer deps are unchanged)
Manual repro from #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

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/913 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 4/26/2026 **Status:** ✅ Merged **Merged:** 4/26/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `claude/laughing-proskuriakova-02bf3d` --- ### 📝 Commits (2) - [`0d6c8b7`](https://github.com/cloudflare/vinext/commit/0d6c8b715be03481b9265dafde915a5ab84b7024) fix(dev): backstop uncaughtException for socket disconnects pipe() misses - [`95cd980`](https://github.com/cloudflare/vinext/commit/95cd98034f444deeb84ce8afbe3d8f90a398f7aa) fix: address bonk review on uncaughtException backstop ### 📊 Changes **1 file changed** (+38 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/index.ts` (+38 -0) </details> ### 📄 Description ## 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 #905 (original issue — re-occurring after #911 partially closed it) - Builds on #911 ## Test plan - [ ] `vp check` passes (note: 12 pre-existing typecheck errors about missing peer deps are unchanged) - [ ] Manual repro from #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](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>