PR #45125 - oven-sh/bun

Problem

A ws client's once() gives one native event to emit() twice. For 'error' the second emit() throws Unhandled error. (ErrorEvent { isTrusted: [Getter] }). jsdom uses once("error"), so a refused connection exits with 1.
#onOrOnce (src/js/thirdparty/ws.js:362) adds a { once: true } closure to the native socket. once() then calls on(), which adds a second.

Fix

Each forwarded event (open, close, message, error, ping, pong) and the handshake reader has one static listener for all clients. #armNativeBridge adds it once per client.
Correct because one native listener is one emit() per event. EventEmitter gives the once behavior.
Verified: test/regression/issue/26358.test.ts (8 of 14 tests fail on main) and the suites in Notes.
Self-reviewed: 28 concerns on the earlier diff, 14 addressed. The other 14 belong to the default change (Notes).

Background

The ws client is an EventEmitter around the native WebSocket. For each event that it forwards, a native listener calls emit().
This is #37972 without closures, as its review asks, with its design and most of its tests.
The listener finds its client in a WeakMap. A symbol property on the native socket would show through event.target.

Downsides

Each forwarded event costs one WeakMap.get. 1,000,000 messages: 2,360 ms on main, 2,386 ms here (medians of 12 interleaved runs, deviation 186 and 303 ms).
No other cost found. Checked: a client with no listener (no change), four on() listeners (261 bytes less), release text (-141 bytes).

Notes

Repro

import { WebSocket } from "ws";
const ws = new WebSocket("ws://127.0.0.1:1/"); // nothing listens on port 1
ws.once("error", error => console.log("listener:", error.message));

	output	exit code
main and 1.4.2	the listener's line, then `error: Unhandled error. (ErrorEvent { isTrusted: [Getter] })`	1
this PR	the listener's line	0
Node v26.3.0 + ws 8.18.3	the listener's line	0

events.once(ws, "open") fails the same way on main, through its own once("error"). With jsdom 25.0.1, new window.WebSocket("ws://127.0.0.1:1/") makes a Bun process print that error and exit with 1 (lib/jsdom/living/websockets/WebSocket-impl.js:163). With this PR it prints nothing, onerror and onclose run, and the exit code is 0.

For the other events nothing throws, but ws.once("message", a); ws.on("message", b) calls b twice for the first message.

What changes, and what does not

on(), once(), addListener(), prependListener() and prependOnceListener() all go through #armNativeBridge. It sets the bit of the event in #eventId and adds the native listener. A second call for the same event adds nothing.
The listeners are still added at the first on() for an event, not in the constructor. The native socket asks hasEventListeners before it dispatches open, message, ping, pong and handshake, so a listener that is always there would change what main delivers.
The listeners are static private functions of the class. They read the client from a module WeakMap that has the native socket as the key. The entry is made when a native listener is added. A later change can remove a listener with removeEventListener, because the function is not a closure. This PR removes none.
noopBridgeListener is gone. #armNativeBridge used it to add and remove a placeholder listener of the emitter.
No default changes. A client with no 'error' listener whose opening handshake fails is silent, as on main. A listener that was added and then removed still gives Unhandled error, as on main.
A finishRequest that returns before req.end() makes the client dial twice (#44123). The listeners of the client stay on the first socket, as on main. One new test holds that.

Cost, measured

linux-x64, release builds of main 7c96a1212f and of the same commit with this change. The client counts are for 1,000 open clients against an in-process Bun.serve.

Per `ws` client	main	this PR
no listener: native listeners added	0	0
no listener: GC cells, GC heap bytes, other bytes	5.38, 410, 53	5.38, 410, 53
`on()` for open, message, close, error: native listeners added	4	4
the same: GC cells, GC heap bytes, other bytes	13.4, 771, 92	5.4, 481, 121
`once("open")`: native listeners added, GC cells	2, 7.41	1, 5.41
`once()` for the same four events: native listeners added, GC cells	8, 31.4	4, 17.4

GC cells and GC heap bytes are the growth of heapStats().objectCount and heapSize for each client. They include the server's socket. Other bytes are the growth of extraMemorySize: the WeakMap entries are there. A client with four on() listeners drops four closures and four scope objects (290 bytes) and gains one entry (29 bytes).

Per process	main	this PR
release text (`size`)	89,151,348	89,151,207 (-141)
bundled `ws.js`	34,853 bytes	34,712 bytes
host functions	no change	no change

Per message: the server sends 1,000,000 text messages of one byte to one client with an on("message") listener. Wall time from the first to the last message, 12 interleaved runs of each build: median 2,360 ms on main and 2,386 ms with this PR (+1.1%), mean 2,334 ms and 2,265 ms (-3.0%), standard deviation 186 ms and 303 ms. The difference is below the noise of this machine. perf and valgrind are not installed here, so there is no instruction count.

Other open PRs on the same lines

#37972 (once() adds a second native listener): this PR is that fix, with static listeners. It can be closed when this one merges.
#36375 (Unexpected server response for error-only listeners): it adds one line to the 'error' arm that this PR removes. After this PR the line goes in #armNativeBridge.
#44122 and #44123 (finishRequest dials once): they change #createWebSocket, not the listeners.
#45122 (every failed handshake of the native socket fires error): separate, in native code.

What this PR was before

Until the push of 0732b8c185 this PR also made an unhandled failed handshake throw, as npm ws does. That default change is not in it any more. It needs a maintainer's yes, and it has gates of its own:

#43711: a Bun server that calls terminate() in 'connection' drops its 101, so its client would throw. The existing test sockets can be terminated fails with the throw until the 101 is sent.
#41698: the message of the native error has the user and the password of the URL.
The handlers of onerror and addEventListener('error') must count as handlers of the client, not of one native socket. They are on the native socket now.
A client that dials before req.end() (#44123) must not throw for either dial.

Self-review

The review was of the diff that this PR had before (the throw, with a capture listener that counted native listeners). It raised 28 concerns. Its answer was this re-cut.

Addressed here, 14: all six events and the handshake reader use the shared listener, not only 'error'. The listeners are added at the first on(), as on main, so the order of handlers is main's order. #eventId stays, with a test for a late req.end(). The lookup is a WeakMap, not a symbol on the native socket. The test sockets can be terminated and the it.todo next to it are as on main again. The branch is on a main that has #45043. The crash fix no longer waits for a decision about a default.

The other 14 are about the default change (handlers counted on one native socket, a second dial that throws, credentials in the thrown message, the 120 s handshake timeout, options that the client does not send). They go with that change.

Suites run with the debug build (linux-x64)

test/regression/issue/26358.test.ts: 14 pass. With the release build of main: 6 pass, 8 fail.
test/js/first_party/ws/ws.test.ts (main's file, not edited): 96 pass, 3 todo. In one of two runs the TLS test ciphers took 5.1 s at its limit of 5 s. It takes the same time with main's sources on this machine.
ws-proxy.test.ts (19), ws-upgrade-events.test.ts (13), ws-syscall-fault.test.ts (3), test/js/node/http/node-http-with-ws.test.ts (25): all pass. The last three ran with a 60 s timeout: each test starts a debug binary.
test/regression/issue/012040.test.ts, 29684.test.ts, 32734.test.ts, 3613.test.ts, 03844/03844.test.ts, test/cli/inspect/inspect.test.ts (29): all pass.
A 48-row script of failed handshakes (listener forms, onerror, addEventListener, close() and terminate() while CONNECTING, a late req.end(), a 404 with 'upgrade' and 'unexpected-response' listeners), release builds: this PR gives main's result on 45 rows. The 3 that differ are once, once then on, and events.once(ws, "open").
A stress run under ASAN: 300 rounds of four clients (no reference kept, failing and opening) with Bun.gc(true) in each round. No ASAN report.