ws: emit client 'unexpected-response' event for non-101 upgrades

[robobun]

Problem

Fixes #31792.

Bun's ws client shim hardcoded the 'unexpected-response' event as "not implemented" and printed a warning instead of firing it. When a server answers a WebSocket upgrade with a non-101 status (e.g. Chrome DevTools replying 401/404 on an unknown /devtools/browser/<id> path), the failed upgrade only surfaced as a generic WebSocket connection ... failed: Expected 101 status code error — not Node's 'unexpected-response' event, and not Node's Unexpected server response: <status> error for the no-listener case.

This breaks puppeteer / chrome-devtools-mcp, which rely on this path (the former via the no-listener Unexpected server response error, the latter via an 'unexpected-response' listener).

Reproduction

const net = require("net");
const WebSocket = require("ws");

// A server that answers the upgrade with a non-101 status
const server = net.createServer(sock =>
  sock.once("data", () => sock.end("HTTP/1.1 401 Unauthorized\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")),
);
server.listen(0, "127.0.0.1", () => {
  const ws = new WebSocket(`ws://127.0.0.1:${server.address().port}/devtools/browser/xxx`);
  ws.on("unexpected-response", (req, res) => { console.log("unexpected-response", res.statusCode); server.close(); });
});

Before: [bun] Warning: ws.WebSocket 'unexpected-response' event is not implemented in bun and the event never fires (an 'error' with Expected 101 status code fires instead).
After (matches Node): unexpected-response 401.

Cause

The native upgrade client (WebSocketUpgradeClient) fast-failed on any non-HTTP/1.1 101 response and discarded the HTTP response (status, headers, body), terminating with a generic Expected101StatusCode. JS had nothing to build (req, res) from, so the shim stubbed the event with a warning.

Fix

• WebSocketUpgradeClient no longer fast-fails on the HTTP/1.1 101 prefix; it parses the full response so the existing status_code != 101 branch in process_response can hand the status/headers/body to JS (via CppWebSocket::did_receive_handshake_response) before terminating. max_http_header_size() still bounds buffering, and a non-HTTP reply still fails with InvalidResponse.
• WebSocket::didReceiveHandshakeResponse (C++) builds a { statusCode, statusMessage, headers, rawHeaders, body } object and dispatches an "unexpected-response" MessageEvent. It early-returns unless an "unexpected-response" listener is registered, so the browser-style new WebSocket() path pays nothing. Headers are passed using the same {ptr,len}-of-picohttp::Header ABI as the fetch header path.
• src/js/thirdparty/ws.js eagerly registers that native listener, and on a non-101 response:
  • with an 'unexpected-response' listener → emits 'unexpected-response'(req, res) with an http.IncomingMessage-like readable res (statusCode/statusMessage/headers/rawHeaders + the body as a stream) and suppresses the native error/close (matches Node — the user owns the connection).
  • without one → emits error Unexpected server response: <status> then close (matches Node's abortHandshake). This is the path puppeteer / chrome-devtools-mcp rely on.

The 'unexpected-response' warning stub is removed; 'upgrade' and 'redirect' remain stubbed (out of scope for this issue).

Verification

New tests in test/js/first_party/ws/ws.test.ts (unexpected-response (non-101 upgrade) describe), using an in-process raw-TCP server so the non-101 bytes are flushed deterministically:

• 'unexpected-response' fires with res.statusCode === 401, res.statusMessage, the response headers, and the body drained via res.on("data"/"end"); no spurious error/close.
• Without a listener, the events are exactly ["error:Unexpected server response: 404", "close:1006"] — verified byte-for-byte identical to Node ws@8.

Both tests pass under the debug (ASAN) build and fail under USE_SYSTEM_BUN=1 (the event never fires; the no-listener case emits the old Expected 101 status code/1002 instead), confirming they exercise the fix. Also verified: the happy-path 101 upgrade, once("unexpected-response"), and a 302 (no followRedirects) surfacing as unexpected-response with res.headers.location — all match Node.

Relationship to #31408 and #5951

#31408 (open, issue #31406) implements the upgrade event for the 101 success path and introduces a WebSocket::didReceiveHandshakeResponse of its own (a handshake event). This PR implements the unexpected-response event for the non-101 failure path — complementary, but it touches the same files (WebSocket.{h,cpp}, CppWebSocket.rs, WebSocketUpgradeClient.rs, ws.js) with a differently-shaped didReceiveHandshakeResponse. Whichever lands second will need a rebase; the two could also be unified onto a single handshake-response callback that drives both events.

#5951 tracks both the 'upgrade' and 'unexpected-response' warnings. This PR only implements the 'unexpected-response' half, so it does not fully close #5951 — the 'upgrade' half is #31408. I've intentionally not added Fixes #5951 so it isn't auto-closed before 'upgrade' also lands.

[coderabbitai]

Walkthrough

This PR implements the unexpected-response event for WebSocket connections when the server responds with a non-101 HTTP status during upgrade. The change spans native Rust/C++ integration and JavaScript event handling, with full test coverage for both listener and fallback scenarios.

Changes

WebSocket unexpected-response event for non-101 upgrades

Layer / File(s)	Summary
FFI contract for handshake response data src/http_jsc/websocket_client/CppWebSocket.rs, src/jsc/bindings/webcore/WebSocket.h	PicoHeaders struct and Rust FFI declarations define how response data (status, headers, body) flows to C++. C++ header declares didReceiveHandshakeResponse method and forward-declares BunString.
Native upgrade client response buffering and forwarding src/http_jsc/websocket_client/WebSocketUpgradeClient.rs, src/http_jsc/websocket_client/CppWebSocket.rs	Upgrade client now buffers full non-101 responses instead of fast-failing, allowing status/headers/body to be forwarded via CppWebSocket::did_receive_handshake_response wrapper that manages VM event loop.
C++ unexpected-response event construction and dispatch src/jsc/bindings/webcore/WebSocket.cpp	WebSocket::didReceiveHandshakeResponse decodes Pico headers into JS objects and constructs an IncomingMessage-like response payload, dispatching it as MessageEvent during CONNECTING state when listeners exist.
JS event handler with fallback error/close behavior src/js/thirdparty/ws.js	Wraps imports Readable, tracks handshake handling state, and implements #onUnexpectedResponse to emit unexpected-response with fabricated stream or schedule synthetic error/close events when no listener exists. Suppresses native close/error to prevent duplicates.
Non-101 upgrade response tests test/js/first_party/ws/ws.test.ts	Tests non-101 responses with and without unexpected-response listeners, validating status/headers/body propagation and error/close fallback behavior.

Suggested reviewers

• Jarred-Sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and concisely describes the main change: implementing the 'unexpected-response' event for non-101 WebSocket upgrade responses, which is the core objective of the PR.
Linked Issues check	✅ Passed	The PR fully addresses issue #31792 by implementing the 'unexpected-response' event for non-101 HTTP responses during WebSocket upgrades, matching Node.js behavior for both listener and no-listener cases.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the 'unexpected-response' event for non-101 WebSocket upgrades. The PR explicitly notes that 'upgrade' and 'redirect' events remain stubbed (out of scope), and does not introduce unrelated modifications.
Description check	✅ Passed	The PR description comprehensively addresses both required template sections with clear problem statement, reproduction steps, detailed cause analysis, and thorough verification methodology.

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

---

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

[robobun]

^{Updated 2:23 AM PT - Jun 4th, 2026}

❌ @autofix-ci[bot], your commit 98297b1 has 2 failures in Build #60293 (All Failures):

• scripts/build/ci.ts - build failed on 🐧 x64-musl - build-cpp
• scripts/build/ci.ts - build failed on 🐧 aarch64-musl - build-cpp
• scripts/build/ci.ts - build failed on 🐧 x64-musl-baseline - build-cpp
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🍎 aarch64 - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64 - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-asan - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-musl-baseline - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🍎 x64 - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 aarch64-android - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-musl - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-android - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on :freebsd: x64 - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-baseline - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 aarch64 - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on :freebsd: aarch64 - build-rust
• linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 aarch64-musl - build-rust

---

🧪   To try this PR locally:

bunx bun-pr 31800

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

bun-31800 --bun

[github-actions]

Found 1 issue this PR may fix:

1. ws.WebSocket 'upgrade' and 'unexpected-response' event is not implemented in bun #5951 - Tracks the missing unexpected-response event on the ws WebSocket client shim, which is exactly what this PR implements

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

Fixes #5951

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. ws: Implement upgrade and unexpected-response events #25777 - Also implements both upgrade and unexpected-response events for the ws WebSocket client shim
2. ws: support upgrade and unexpected-response events #28114 - Also implements both upgrade and unexpected-response events for the ws WebSocket client shim

🤖 Generated with Claude Code

[robobun]

Closing in favor of #28114, which implements both upgrade and unexpected-response and is a strict superset of this change — Node-faithful header coalescing, multi-read/EOF body buffering, reentrant-close hardening, and addEventListener('error')/onerror suppression (this PR only suppressed the native error on the .on('error') path, so puppeteer's ws.addEventListener('error') would still have seen the old Expected 101 error). Commented there noting it also resolves #31792.

[claude]

🟡 The body passed to 'unexpected-response' is best-effort — remain_buf only contains bytes that arrived in the same TCP read(s) as the headers, with no Content-Length/chunked handling before terminate() closes the socket. If a server's non-101 body spans multiple packets (e.g. a large 401 error page), res.on('data') will yield a truncated body, whereas Node's ws hands over a live http.IncomingMessage that streams the full body. Not a blocker (the motivating puppeteer/chrome-devtools-mcp paths only need res.statusCode, and this is strictly better than before), but worth a code comment noting the body is best-effort.

Extended reasoning...

What

The body delivered to the 'unexpected-response' listener may be incomplete. In handle_data (WebSocketUpgradeClient.rs:972), once picohttp::Response::parse sees the terminating \r\n\r\n it returns Ok, and remain_buf = body[bytes_read..].to_vec() captures only the bytes that happened to arrive in the same TCP read(s) as the headers. process_response then immediately hands remain_buf to JS and calls terminate(), which closes the socket — there is no Content-Length parsing, no chunked-encoding handling, and no continued reading. On the JS side, #onUnexpectedResponse does res.push(body); res.push(null), so 'end' fires after whatever partial bytes were captured.

Node's ws differs here: it hands the listener the actual http.IncomingMessage backed by the live socket, which continues streaming until the server closes or Content-Length is satisfied.

Step-by-step example

1. Client sends the upgrade request to a server that replies with HTTP/1.1 401 Unauthorized plus a 16 KB HTML error page and Content-Length: 16384.
2. The server's TCP stack sends the headers + the first ~1.4 KB of body in one segment; the remaining ~14.6 KB follow in subsequent segments.
3. handle_data is invoked with the first segment. picohttp finds \r\n\r\n, returns Ok with bytes_read = header length. remain_buf = the ~1.4 KB body prefix.
4. process_response sees status_code != 101, calls did_receive_handshake_response(..., remain_buf) → JS builds a Readable, pushes the 1.4 KB, then pushes null.
5. terminate() closes the socket; the remaining 14.6 KB are never read.
6. The user's res.on('data') handler observes a 1.4 KB body and res.on('end') fires — silently truncated relative to Content-Length.

Why nothing prevents it

The buffering loop in handle_data waits only for complete headers (ShortRead → buffer and return). Once headers are complete it proceeds unconditionally; nothing inspects Content-Length or Transfer-Encoding to decide whether more body bytes are expected. The new test writes headers + body in a single socket.end(...) call (small enough to coalesce into one packet on loopback), so it doesn't exercise the multi-packet case.

Impact

Low. The PR's stated goal — surfacing res.statusCode / res.statusMessage / res.headers for puppeteer and chrome-devtools-mcp (#31792) — works correctly regardless of body completeness; the no-listener path only uses statusCode in the error message. Realistic non-101 responses to WebSocket upgrades (CDP 401/404, proxy 407, redirects) have tiny or empty bodies that fit in one packet. And before this PR there was no event and no body at all, so any body bytes are a strict improvement. A consumer that actually needs the full error body (rare) would observe truncation, but this is an edge case.

Suggested fix

Full Node-compatible body streaming would require keeping the socket open past process_response, parsing Content-Length/chunked, and pumping further reads into the JS Readable — a substantial restructuring that's reasonably out of scope here. For this PR, a code comment at the did_receive_handshake_response call site (and/or in #onUnexpectedResponse) noting that the body is best-effort — "only bytes that arrived with the headers; not Content-Length-aware" — would document the known limitation. A follow-up issue could track full body streaming if a real consumer needs it.