net: route throws from socket lifecycle listeners to uncaughtException

RO
2mo ago
+257-10

Part of #29761.

When a user listener attached to a net.Socket / net.Server lifecycle event throws synchronously, the throw unwinds back into the native socket callback, which catches it and funnels it into the socket's onError handler (the 'error' event) or, on the open path, tears the connection down. Node instead re-raises the throw as 'uncaughtException' and leaves the socket alive.

Fix

Add a safelyInvokeListeners(fn) helper that wraps a group of consecutive self.emit(...) calls and forwards a caught throw through reportError (which surfaces as 'uncaughtException'). Wrapping a whole emit group (not each emit) preserves Node's short-circuit semantics: a throw from the 'connect' listener aborts the synchronous sequence so 'ready' never fires. reportError is captured at module load so the routing can't be defeated by user code clobbering globalThis.reportError.

Applied to the non-TLS lifecycle paths a user listener reaches synchronously from a native socket callback:

  • open: SocketHandlers.open 'connect'/'ready'; afterConnect 'connect'/'ready'
  • server accept: onconnection 'connection'
  • onread.callback: reportError instead of self.emit('error')

Scope: the 'data' listener case is deferred

The original report is about a throwing 'data' listener. An earlier revision fixed that by wrapping self.push(buffer) in the data handlers (pushDataToSocket), but CI showed it breaks node:http:

  • test-http-abort-client, test-http-client-aborted-event, test-http-server-capture-rejections: a Parse Error: Invalid character in chunk size surfaced as uncaughtException instead of the client res 'error' (ECONNRESET).
  • test-http-catch-uncaughtexception: hang/timeout.

All four pass on main and failed with the data-path wrapping. Root cause: http's client socketOnData is itself a socket.on('data') listener, and Bun's native HTTP parser throws parse errors where Node returns them; http relies on the native layer catching that throw and routing it to the socket 'error' event. Wrapping self.push at the JS layer intercepts that routing, and catching mid-Readable.push() also corrupts stream state (the hang). There is no way to distinguish a user 'data' listener throw from an internal consumer (the http parser) throw at the net push layer.

A correct 'data'-path fix needs a coordinated change in the native socket onData path (routing a genuine data-callback throw to uncaughtException after the Readable unwinds, matching Node's C++ boundary) plus http's parser returning parse errors inline, and it also affects the Bun.connect API surface. That is an architectural decision left for a follow-up; this PR ships the safe, same-bug-class lifecycle fixes.

Verification

New describe.concurrent("uncaughtException in socket listener") block in test/js/node/net/node-net.test.ts:

  • client 'connect' throw → uncaughtException; socket stays writable (Node leaves it alive).
  • client 'ready' throw → uncaughtException.
  • server 'connection' throw → uncaughtException.
  • onread.callback throw → uncaughtException.
  • connect→ready short-circuit: a throw from 'connect' aborts the emit group so 'ready' never fires.

Each test fails with net.ts reverted to main and passes with the fix. A 40-test node:http parallel sweep stays clean (only the pre-existing agent-keepalive flake, which also fails on main).

no test proof · iteration 14 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/net/node-net.test.ts