node:net: route throws from 'data'/'connection' listeners to uncaughtException and keep the socket alive

Merged
RO
2mo ago
+318-9

Fixes #29761

Problem

  • A throw inside a net.Socket 'data' listener or a net.Server 'connection' listener goes to the socket 'error' listener and the socket closes. Node reports uncaughtException, keeps the socket reading, and delivers the next chunk.
  • Cause: src/js/node/net.ts calls self.push(buffer) and self.emit("connection") bare inside native callbacks. push() runs the 'data' listeners synchronously. The throw unwinds into the native dispatch, which routes it to the handler table's error entry, the path for transport failures.

Fix

  • pushDataToSocket wraps self.push(buffer) in the three handler tables with a try/catch that calls reportError(e) and leaves the socket alone. onconnection wraps the 'connection' emit the same way and still starts the read.
  • Matches Node: onStreamRead calls stream.push() bare, so the throw becomes uncaughtException and nothing stops the handle. Node also skips readStop on a throw. Safe here too: a throw only comes from the flowing path, where push() returns true.
  • Transport failures do not pass through push(), so their error routing is unchanged. Bun.listen and Bun.connect are unchanged.
  • Verified: test/js/node/net/node-net.test.ts runs seven fixture files (listener-throw-*-fixture.js: plain and TLS 'data' throws on both sides, a 'connection' throw, the pg pool shape from #34064, no handler) with bunRun. All seven fail on main, pass here, and print the same expected logs under node v26.3.0 unchanged. The net, http and tls suites pass (Notes).

Background

  • node:net is a JS Duplex over a native Bun.connect/Bun.listen socket. A handler table receives the native callbacks.
  • The native dispatch applies the public contract: a throwing handler is reported through the table's error handler. node:net has to catch user throws at the JS boundary.
  • reportError(e) runs the uncaughtException handlers synchronously, or prints the error and exits 1 when there is none. net.ts already uses it for the TLS handshake emits and onread.
Notes

Repro (server side). Node: ["connection","data:A","UNCAUGHT:data-boom","data:B","close:false"]. Bun before: ["connection","data:A","socket-error-event:data-boom","close:true"].

const net = require('node:net');
const ev = [], fin = () => { console.log(JSON.stringify(ev)); process.exit(0); };
process.on('uncaughtException', e => ev.push('UNCAUGHT:' + e.message));
const srv = net.createServer(s => {
  ev.push('connection');
  s.on('error', e => ev.push('socket-error-event:' + e.message));
  s.on('data', d => { ev.push('data:' + d); if (String(d) === 'A') throw new Error('data-boom'); });
  s.on('close', h => { ev.push('close:' + h); srv.close(); setTimeout(fin, 80); });
});
srv.listen(0, '127.0.0.1', () => {
  const c = net.connect(srv.address().port, '127.0.0.1', () => {
    c.write('A'); setTimeout(() => c.write('B'), 120); setTimeout(() => c.end(), 300);
  });
  c.on('error', () => {});
});

The script from #29761 (with listen(0)) now prints [Fatal] Uncaught Exception: nonExistentFunction is not defined and exits 1, the same as Node.

reportError is captured at module load (const reportError = globalThis.reportError) so user code that clobbers the global cannot defeat the routing.

History of this PR. The first revision also carried a uws_res_end_without_body guard and a node-http.test.ts test. Routing the throw correctly surfaced a second bug: ServerResponse.destroy() on an in-flight chunked response wrote Connection: close\r\n\r\n into the body, and the client parse error that produced had been swallowed by the same misrouting. #39364 fixed that on main with its own tests (node-http-transfer-encoding.test.ts), so this PR no longer touches libuwsockets.cpp.

http client parse errors: _http_client.ts gets parse errors as a return value from parser.execute(), not as a throw, so they do not pass through the push() catch. Probe: a server that answers Transfer-Encoding: chunked with ZZZ as the chunk size gives ["response:200","req-error:HPE_INVALID_CHUNK_SIZE","res-aborted","req-close","res-error:ECONNRESET"] on Node and on this branch.

Other listener paths probed against Node v26.3.0: 'end' and 'close' throws already match (both emit from a JS tick). A client 'connect'/'ready' throw is still routed to the socket 'error' listener. That is the scope of #29762 and is not changed here.

Sweeps on this branch (debug build, linux x64):

  • test/js/node/net/node-net.test.ts: 95 pass, 10 fail. The same 10 fail with main's net.ts in this container: localhost resolves to ::1 first, and there is no outbound DNS.
  • test/js/node/http/node-http.test.ts: 149 pass, 1 fail (request via http proxy, same localhost cause, fails on main).
  • test/js/node/tls/node-tls-server.test.ts 76/77 (same cause), node-tls-connect.test.ts 45/45, node-tls-cert.test.ts 28/28, node-http-transfer-encoding.test.ts 36/36.
  • test/js/node/test/parallel/test-net-*.js: 139/139 (test-net-pingpong flaked under 4x parallel load, passes alone).
  • test-http-*.js: 384/386. test-http-agent-keepalive and test-http-client-timeout-option fail with main's net.ts too. test-http-abort-client, test-http-client-aborted-event, test-http-server-capture-rejections, test-http-catch-uncaughtexception pass.
  • test-tls-*.js: 185/186 (test-tls-client-allow-partial-trust-chain needs bun test, a runner artifact).

Related: #34066 took the other approach (wrap every handler table entry, then socket.terminate() on a caught throw). Node leaves the socket alive, so that teardown is the wrong semantics for this bug. #29762 covers the lifecycle emits ('connect'/'ready').

no test proof · iteration 1 · 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