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

[robobun]

Fixes #29761

Problem

An exception thrown from a user 'data' listener on a net.Socket, or from a net.Server 'connection' listener, unwinds into the native socket dispatch, which catches it and routes it to the socket handler table's error entry. That entry exists for transport failures (ECONNRESET, fatal write), so a programming error in a user listener was misclassified as a socket error: it surfaced as socket.emit('error', err) and the connection was torn down, or was silently dropped when the error handler returned early, so the ubiquitous sock.on('error', () => {}) hid it. Node surfaces the throw as process.on('uncaughtException') and leaves the socket reading, so the next chunk is still delivered.

// node: ["connection","data:A","UNCAUGHT:data-boom","data:B","close:false"]
// bun:  ["connection","data:A","socket-error-event:data-boom","close:true"]  (client side)
//       ["connection","data:A","data:B","close:false"]                       (server side: swallowed)
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', () => {});
});

Same for a throw inside the createServer(handler) connection handler: even with no error listener the accepted connection is closed where Node keeps it established.

Fix

Wrap Readable.push() in the three data handler tables (SocketHandlers / ServerHandlers / SocketHandlers2) and the 'connection' emit in onconnection with a try/catch that reports the throw via reportError and leaves the socket alone. The native error routing still applies to actual transport failures because those do not flow through push(). The public Bun.listen/Bun.connect API is unchanged.

uws_res_end_without_body guard

The correct routing surfaces a second bug the misrouting was masking: uws_res_end_without_body wrote a Connection: close header and the header-terminating CRLF even after res.write() had started the body, so ServerResponse.destroy() on an in-flight chunked response injected Connection: close\r\n\r\n into the middle of the body. The client then parses that as HPE_INVALID_CHUNK_SIZE, and req.emit('error', parseErr) with no listener rethrows; previously that rethrow was itself swallowed by the same native catch. Gate those writes on HTTP_WRITE_CALLED so an abort after the body started emits nothing (Node sends nothing extra on destroy). This is the uws_res_end_without_body hunk of #32036.

Verification

test/js/node/net/node-net.test.ts: four subprocess tests covering server-side 'data' listener throw, client-side 'data' listener throw, 'connection' listener throw (each reaches uncaughtException, no socket 'error' fires, the next chunk is delivered, close(hadError) is false), and the no-handler case (process exits 1 with the thrown error on stderr). Expected outputs taken verbatim from Node v26.3.0; all four fail on main.

test/js/node/http/node-http.test.ts: destroying a chunked response mid-stream writes exactly the one chunk frame with no appended header bytes. Fails on main.

Regression sweeps against this branch, failures match main:

• test/js/node/test/parallel/test-http-*.js: 372/377 pass (5 pre-existing, including test-http-abort-client / test-http-client-aborted-event / test-http-catch-uncaughtexception / test-http-server-capture-rejections which all pass)
• test/js/node/test/parallel/test-net-*.js: 137/140 pass (3 pre-existing)
• test/js/node/net/node-net.test.ts, test/js/node/http/node-http.test.ts, test/js/node/tls/node-tls-{server,connect}.test.ts, test/js/bun/http/serve.test.ts: no new failures

Scope: TLS 'secureConnection' emit intentionally excluded

The SSL sibling of the 'connection' emit, server.emit("secureConnection", self) in ServerHandlers.handshake, is left unwrapped. Wrapping it regresses test/js/node/test/parallel/test-tls-close-error.js: Bun currently fires secureConnection for a client whose verify-reject will tear the connection down (Node does not; the test's handler is common.mustNotCall()), and that assertion throw is today silently routed through the native error handler. Surfacing it as uncaughtException fails the test. That is a pre-existing bug in when Bun fires secureConnection, tracked separately. The TLS 'data' path is covered: accepted TLS sockets use ServerHandlers.data, which is wrapped via pushDataToSocket, and a throwing TLS 'data' listener now matches Node.

Related

• net: route throws from socket lifecycle listeners to uncaughtException #29762 wraps the 'connect'/'ready'/'connection' emit groups but explicitly deferred the 'data' path because it unmasked the uws_res_end_without_body mid-body injection above. This PR covers the 'data' and 'connection' cases and includes the guard that blocked it.
• node:net: route exceptions thrown by socket event listeners to uncaughtException #34066 wraps every handler table entry but calls socket.terminate() on a caught throw; Node leaves the socket alive and delivers the next chunk, so that teardown is the wrong semantics for this bug.
• Don't write Connection: close into an in-flight chunked response body #32036 is the broader mid-body-header fix; this PR carries only the uws_res_end_without_body hunk that the net change depends on.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e45369d0-26bf-449f-a7f0-0a72177c6cef

📥 Commits

Reviewing files that changed from the base of the PR and between 7144ce0 and 73454fa.

📒 Files selected for processing (4)

• src/js/node/net.ts
• src/uws_sys/libuwsockets.cpp
• test/js/node/http/node-http.test.ts
• test/js/node/net/node-net.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 8:36 PM PT - Jul 23rd, 2026}

❌ @robobun, your commit 73454fa has 1 failures in Build #79121 (All Failures):

• test/js/bun/spawn/spawn.test.ts - code 1 on 🐧 25.04 x64
• test/js/bun/spawn/spawn.test.ts - code 1 on 🐧 13 x64
• test/js/bun/spawn/spawn.test.ts - code 1 on 🐧 3.23 x64

---

🧪   To try this PR locally:

bunx bun-pr 35347

That installs a local version of the PR into your bun-35347 executable, so you can run:

bun-35347 --bun

[github-actions]

Found 1 issue this PR may fix:

1. ERR_INCOMPLETE_CHUNKED_ENCODING with Next.js app using Bun in Docker behind NGINX proxy #19789 - The uws_res_end_without_body fix prevents Connection: close header bytes from being injected into a chunked response body after res.write() has started, which corrupts the chunked stream and causes ERR_INCOMPLETE_CHUNKED_ENCODING / HPE_INVALID_CHUNK_SIZE errors behind reverse proxies like NGINX.

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #19789

🤖 Generated with Claude Code

[robobun]

On #19789: the uws_res_end_without_body guard does remove one real source of mid-body chunked-stream corruption (header bytes injected when an in-flight chunked response is aborted), which is consistent with ERR_INCOMPLETE_CHUNKED_ENCODING behind a proxy, but that issue has no minimal reproduction (Next.js + Docker + NGINX, and the reporter's workaround was swapping Docker base images while still running bun). Not marking it fixed without a verified repro; leaving it for a follow-up check once this lands.

[github-actions]

This PR may be a duplicate of:

1. net: route throws from socket lifecycle listeners to uncaughtException #29762 - Same goal of routing throws from socket lifecycle listeners to uncaughtException instead of crashing
2. node:net: route exceptions thrown by socket event listeners to uncaughtException #34066 - Nearly identical goal of routing exceptions from socket event listeners to uncaughtException, touches the same files (node/net.ts)
3. Don't write Connection: close into an in-flight chunked response body #32036 - Overlaps with the uws_res_end_without_body fix to prevent corrupting in-flight chunked responses

🤖 Generated with Claude Code

[robobun]

Not a duplicate; intentional overlap spelled out in the Related section of the description:

• net: route throws from socket lifecycle listeners to uncaughtException #29762 ships the 'connect'/'ready'/'connection' lifecycle cases only and explicitly deferred the 'data' path because wrapping it unmasked the mid-body header injection in three test-http-abort-* tests. This PR covers 'data' and 'connection' and carries the guard that blocked the 'data' half; net: route throws from socket lifecycle listeners to uncaughtException #29762's remaining 'connect'/'ready' cases are not in this diff.
• node:net: route exceptions thrown by socket event listeners to uncaughtException #34066 wraps every handler table entry and calls socket.terminate() on a caught throw. Node leaves the socket alive and delivers the next chunk, so tearing the socket down is the wrong semantics; this PR keeps the socket reading (the new tests assert close:false and the follow-up chunk).
• Don't write Connection: close into an in-flight chunked response body #32036 is the broader mid-body-header fix (also touches HttpResponse.h's internalEnd); this PR carries only the uws_res_end_without_body hunk that the net change depends on. Whichever lands first, the other rebases trivially.