[PR #915] [CLOSED] fix(dev): patch EventEmitter.emit to swallow peer-disconnect errors #945

New issue

Closed

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

BreizhHardware commented

2026-05-06 13:11:00 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/915
Author: @james-elicx
Created: 4/26/2026
Status: ❌ Closed

Base: main ← Head: claude/dev-socket-emit-patch

📝 Commits (1)

7894815 fix(dev): patch EventEmitter.emit to swallow peer-disconnect errors

📊 Changes

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

View changed files

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

📄 Description

Summary

Third pass at #905. Both #911 (per-connection socket listener) and #913 (process-level uncaughtException backstop) left a hole — the dev server still crashes in this exact shape after a successful response, with both prior fixes installed:

node:events:487
      throw er; // Unhandled 'error' event
Error: read ECONNRESET
    at TCP.onStreamRead
Emitted 'error' event on Socket instance at:
    at Socket.onerror (node:internal/streams/readable:1035:14)   // pipe()
    at Socket.emit (node:domain:489:12)                           // domain wrap
    at emitErrorNT (node:internal/streams/destroy:170:8)

Two things in that trace defeat the previous fixes:

Socket.onerror at readable.js:1035 is Node's pipe() re-emission. When a piped source errors and the destination has no 'error' listener, pipe() emits the error on the destination — and the destination here is not the inbound connection socket the per-connection guard attached to.
Socket.emit at node:domain:489 is the node:domain wrapper. Errors routed through a domain bypass uncaughtException entirely, so the process-level handler from #913 never runs.

Three real call sites in vinext dev hit this:

fromWeb(fetch().body).pipe(res) in proxyExternalRewriteNode
streaming surfaces inside @vitejs/plugin-rsc with their own pipe topology
outbound sockets created by middleware fetch()

Approach

The only place to catch every variant is at the synchronous source: EventEmitter.prototype.emit. Wrap it once, short-circuit only when all three of these hold:

type === 'error'
the error has a peer-disconnect code (ECONNRESET / EPIPE / ECONNABORTED)
the emitter has zero 'error' listeners (the exact condition under which Node would throw)

Anything else passes through untouched — including non-peer-disconnect errors, and peer-disconnect errors on emitters that already have a listener (so registered listeners still receive them).

Installed once per process via a Symbol.for guard so dep re-optimization, full reloads, and repeated plugin invocations can't double-install or tear it down.

The belt-and-braces uncaughtException / unhandledRejection handlers from #913 stay in place for surfaces that bypass EventEmitter.emit entirely (raw promise rejections, native callbacks). Both still sync-throw on non-peer-disconnect errors to preserve Node's default crash semantics.

Verification

Behavior matrix locally:

$ node -e '/* installed patch as in PR */
  const ee = new (require("events").EventEmitter)();
  const reset = Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" });
  ee.emit("error", reset); console.log("survived peer disconnect");
  try { ee.emit("error", new Error("real bug")); }
  catch (e) { console.log("real bug still throws:", e.message); }
  ee.on("error", e => console.log("listener got:", e.code));
  ee.emit("error", reset);
'
survived peer disconnect
real bug still throws: real bug
listener got: ECONNRESET

Dev-only by virtue of running inside the Vite plugin factory; not invoked in production (Cloudflare Worker).

Refs

Refs #905 (re-occurring after #911 and #913)
Builds on #911, #913

Test plan

vp check passes (one pre-existing benchmarks Cannot find module 'vinext' error is unrelated)
Reproducer from #905 with proxyExternalRewriteNode/RSC streaming: dev server no longer crashes after browser disconnect mid-stream, including across dep re-optimization
Genuine programming errors still crash dev server with full stack
Existing 'error' listeners on streams still receive events normally

🤖 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/915 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 4/26/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `claude/dev-socket-emit-patch` --- ### 📝 Commits (1) - [`7894815`](https://github.com/cloudflare/vinext/commit/789481502d35f0f080fc711f08bbe43be1f04561) fix(dev): patch EventEmitter.emit to swallow peer-disconnect errors ### 📊 Changes **1 file changed** (+84 additions, -38 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/index.ts` (+84 -38) </details> ### 📄 Description ## Summary Third pass at #905. Both #911 (per-connection socket listener) and #913 (process-level `uncaughtException` backstop) left a hole — the dev server still crashes in this exact shape after a successful response, with both prior fixes installed: ``` node:events:487 throw er; // Unhandled 'error' event Error: read ECONNRESET at TCP.onStreamRead Emitted 'error' event on Socket instance at: at Socket.onerror (node:internal/streams/readable:1035:14) // pipe() at Socket.emit (node:domain:489:12) // domain wrap at emitErrorNT (node:internal/streams/destroy:170:8) ``` Two things in that trace defeat the previous fixes: 1. **`Socket.onerror` at `readable.js:1035`** is Node's `pipe()` re-emission. When a piped source errors and the destination has no `'error'` listener, `pipe()` emits the error on the destination — and the destination here is *not* the inbound connection socket the per-connection guard attached to. 2. **`Socket.emit` at `node:domain:489`** is the `node:domain` wrapper. Errors routed through a domain bypass `uncaughtException` entirely, so the process-level handler from #913 never runs. Three real call sites in vinext dev hit this: - `fromWeb(fetch().body).pipe(res)` in `proxyExternalRewriteNode` - streaming surfaces inside `@vitejs/plugin-rsc` with their own pipe topology - outbound sockets created by middleware `fetch()` ## Approach The only place to catch every variant is at the synchronous source: `EventEmitter.prototype.emit`. Wrap it once, short-circuit only when **all three** of these hold: - `type === 'error'` - the error has a peer-disconnect code (`ECONNRESET` / `EPIPE` / `ECONNABORTED`) - the emitter has zero `'error'` listeners (the exact condition under which Node would throw) Anything else passes through untouched — including non-peer-disconnect errors, and peer-disconnect errors on emitters that already have a listener (so registered listeners still receive them). Installed once per process via a `Symbol.for` guard so dep re-optimization, full reloads, and repeated plugin invocations can't double-install or tear it down. The belt-and-braces `uncaughtException` / `unhandledRejection` handlers from #913 stay in place for surfaces that bypass `EventEmitter.emit` entirely (raw promise rejections, native callbacks). Both still sync-throw on non-peer-disconnect errors to preserve Node's default crash semantics. ## Verification Behavior matrix locally: ``` $ node -e '/* installed patch as in PR */ const ee = new (require("events").EventEmitter)(); const reset = Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" }); ee.emit("error", reset); console.log("survived peer disconnect"); try { ee.emit("error", new Error("real bug")); } catch (e) { console.log("real bug still throws:", e.message); } ee.on("error", e => console.log("listener got:", e.code)); ee.emit("error", reset); ' survived peer disconnect real bug still throws: real bug listener got: ECONNRESET ``` Dev-only by virtue of running inside the Vite plugin factory; not invoked in production (Cloudflare Worker). ## Refs - Refs #905 (re-occurring after #911 and #913) - Builds on #911, #913 ## Test plan - [ ] `vp check` passes (one pre-existing benchmarks `Cannot find module 'vinext'` error is unrelated) - [ ] Reproducer from #905 with `proxyExternalRewriteNode`/RSC streaming: dev server no longer crashes after browser disconnect mid-stream, including across dep re-optimization - [ ] Genuine programming errors still crash dev server with full stack - [ ] Existing `'error'` listeners on streams still receive events normally 🤖 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>