fix(node:http): socket.write not sending data in upgrade event handler

[robobun]

Summary

• Enable streaming on the socket before emitting the "upgrade" event, matching the existing behavior for CONNECT requests. Without this, handle.ondrain was undefined so _write() silently skipped calling handle.write().
• Skip socket.cork() for upgrade connections since the socket is handed off to user code and data should flow immediately.

Root Cause

In src/js/node/_http_server.ts, socket[kEnableStreaming](false) was called for all non-CONNECT requests (setting handle.ondrain = undefined). When the "upgrade" event was then emitted, user code calling socket.write() would hit the _write method which requires handle.ondrain to be truthy before calling handle.write(). Since ondrain was undefined, the write was silently skipped and the callback fired as if it succeeded — but no data was ever sent.

The CONNECT handler already had the correct behavior: calling socket[kEnableStreaming](true) before emitting the event.

Test plan

• Added regression tests in test/regression/issue/09882.test.ts
• Verified all 3 tests fail with USE_SYSTEM_BUN=1 (system bun)
• Verified all 3 tests pass with bun bd test (debug build with fix)

Closes #9882

🤖 Generated with Claude Code

[robobun]

^{Updated 3:03 PM PT - Mar 16th, 2026}

❌ @Jarred-Sumner, your commit 15dfc70 has 2 failures in Build #39752 (All Failures):

• test/integration/next-pages/test/dev-server.test.ts - code 1 on 🍎 13 aarch64
• test/js/bun/util/v8-heap-snapshot.test.ts - SIGKILL on 🐧 25.04 x64

---

🧪   To try this PR locally:

bunx bun-pr 27237

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

bun-27237 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3d552846-3220-47ac-855d-bf7c1cd8af2e

📥 Commits

Reviewing files that changed from the base of the PR and between a5a2571 and 15dfc70.

📒 Files selected for processing (1)

• src/js/node/_http_server.ts

---

Walkthrough

Enable streaming on sockets during HTTP upgrades and avoid corking upgrade sockets; add regression tests verifying writes sent during upgrade handlers are transmitted to clients.

Changes

Cohort / File(s)	Summary
HTTP Server Upgrade Handling src/js/node/_http_server.ts	Call socket[kEnableStreaming](true) for upgrade paths before emitting "upgrade" and restrict socket.cork() to non-upgrade requests so upgrade sockets are not corked.
HTTP Upgrade Regression Tests test/regression/issue/09882.test.ts	Add three tests validating data written in upgrade handlers is delivered: single write with callback, 101 response with payload, and multiple sequential socket.write calls before close.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main bug fix: enabling socket.write functionality in upgrade event handlers, which is the primary change in this PR.
Description check	✅ Passed	The description comprehensively covers the summary, root cause, test plan, and references the closing issue, exceeding the basic template requirements.
Linked Issues check	✅ Passed	The code changes directly address issue #9882: enabling streaming before emitting the upgrade event and skipping cork for upgrades now allows socket.write to function properly in upgrade handlers.
Out of Scope Changes check	✅ Passed	All changes in src/js/node/_http_server.ts and the new test file are directly scoped to fixing the socket.write issue in upgrade event handlers as specified in #9882.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

📝 Coding Plan

• Generate coding plan for human review comments

---

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

Tip

CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.

Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present.

[claude]

✅ a5a25 — Looks good!

Reviewed 2 files across src/js/node/ and test/regression/issue/: Fixes socket.write() silently dropping data in HTTP upgrade event handlers by enabling streaming before emitting the "upgrade" event and skipping socket.cork() for upgrade connections.

[claude]

🟣 Pre-existing: The upgrade handler (line 608) is missing a listenerCount("upgrade") guard that the CONNECT handler (line 538) already has — upgrade requests with no listeners will now enable streaming on an unmanaged socket and leak the internal promise. Additionally, assignSocketInternal at line 614 stores the close callback via setCloseCallback(socket, ...), but NodeHTTPServerSocket never calls callCloseCallback(this) on close (unlike ServerResponse.prototype.emit), so the internal promise returned from onNodeHTTPRequest hangs forever for upgrade connections. The CONNECT handler avoids both issues by checking listenerCount and using socket.once("close", resolve) directly.

Extended reasoning...

Bug 1: Missing listenerCount check

The CONNECT handler at line 538 checks server.listenerCount("connect") > 0 before enabling streaming and emitting the event. If no listeners exist, it closes the socket handle. The upgrade handler at line 608 has no equivalent guard — it unconditionally calls socket[kEnableStreaming](true) and emits the "upgrade" event regardless of whether any listeners are registered.

In Node.js's reference implementation (_http_server.js), when there are no upgrade listeners, socket.destroy() is called to clean up the connection. Without this guard in Bun, upgrade requests to a server with no "upgrade" listener will set up ondata/ondrain handlers on a socket that nobody is managing, and the internal promise will hang forever.

Before this PR, the no-listener case was relatively inert because streaming was disabled (writes silently failed). After this PR, socket[kEnableStreaming](true) at line 609 is called unconditionally, making the leak slightly worse by activating event handlers on the orphaned socket.

Bug 2: Internal promise never resolves for upgrade connections

The promise lifecycle for upgrade connections is broken. Here's the intended chain:

1. setCloseCallback(http_res, onClose) at line 602 stores the promise-resolving onClose as http_res[kCloseCallback]
2. assignSocketInternal(http_res, socket) at line 614 stores onServerResponseClose as socket[kCloseCallback]
3. When the socket closes, callCloseCallback(socket) should fire → onServerResponseClose → emitCloseNT(http_res) → callCloseCallback(http_res) → onClose() → resolve

However, this chain is broken because NodeHTTPServerSocket extends Duplex and does not override emit() to intercept the "close" event. Only ServerResponse.prototype.emit (line 1631) calls callCloseCallback(this) when it sees a "close" event. When the socket's #onClose() method fires (line 899), it does not invoke callCloseCallback(socket), so onServerResponseClose is never called.

For normal requests this doesn't matter because resolution goes through http_res.end() → http_res.emit("close") → callCloseCallback(http_res) → onClose(). But for upgrade connections, user code works with the raw socket and never calls http_res.end().

Proof by comparison with CONNECT handler

The CONNECT handler at lines 540-544 correctly handles both issues:

socket[kEnableStreaming](true);
const { promise, resolve } = $newPromiseCapability(Promise);
socket.once("close", resolve);  // Direct binding — no broken callback chain
server.emit("connect", http_req, socket, head);
return promise;

It checks listenerCount first, and ties promise resolution directly to the socket's "close" event via .once(), bypassing the assignSocketInternal/callCloseCallback mechanism entirely.

Impact

Both issues are pre-existing (the upgrade branch with assignSocketInternal and without listenerCount existed before this PR). However, this PR makes upgrade connections actually functional (writes now work), which means more users will exercise this code path and encounter these issues. The fix should mirror the CONNECT handler: guard with listenerCount("upgrade") > 0, call socket.destroy() when there are no listeners, and use socket.once("close", resolve) for promise resolution instead of relying on assignSocketInternal.

[robobun]

Superseded by #30664 — the kEnableStreaming piece here is necessary but insufficient on its own; #30664 also flips isConnectRequest in uWS so post-upgrade reads route through ondata instead of being parsed as HTTP.