fix(websocket): guard stale events and disconnect race in JS client

Two subtle race conditions in the browser WebSocket client:

1. Stale-event clobber. When connect() is called while the old socket is
   in CLOSING state, the readyState guard falls through and a new socket
   is assigned to this.ws. The old socket's queued close event then
   nulls out this.ws, silently breaking send() until the next reconnect.
   Same risk for delayed open/error/message handlers.

2. Reconnect-after-disconnect. clearTimeout() does not cancel a callback
   that has already fired but whose macrotask has not yet run. If
   disconnect() lands in that window, the queued reconnect callback
   still calls #openSocket() and resurrects the connection.

Every event handler now bails out if this.ws no longer points at the
socket that fired the event, and the reconnect timer callback re-checks
shouldReconnect before opening a new socket.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
MHSanaei
2026-05-08 00:00:10 +02:00
parent 10ebc6cbdc
commit b84b58ef21

View File

@@ -104,15 +104,25 @@ class WebSocketClient {
} }
this.ws = socket; this.ws = socket;
// Every handler must check `this.ws !== socket` first. A previous socket
// can still fire events (especially `close`) after we've moved on to a
// new one — e.g. connect() called while the old socket is in CLOSING
// state. Without the guard, a stale close would null out the freshly
// opened socket and silently break send().
socket.addEventListener('open', () => { socket.addEventListener('open', () => {
if (this.ws !== socket) return;
this.isConnected = true; this.isConnected = true;
this.reconnectAttempts = 0; this.reconnectAttempts = 0;
this.#emit('connected'); this.#emit('connected');
}); });
socket.addEventListener('message', (event) => this.#onMessage(event)); socket.addEventListener('message', (event) => {
if (this.ws !== socket) return;
this.#onMessage(event);
});
socket.addEventListener('error', (event) => { socket.addEventListener('error', (event) => {
if (this.ws !== socket) return;
// Browsers fire 'error' before 'close' on failure. We surface it for // Browsers fire 'error' before 'close' on failure. We surface it for
// consumers (so polling fallbacks can engage) but don't log every blip // consumers (so polling fallbacks can engage) but don't log every blip
// — bad networks would flood the console otherwise. // — bad networks would flood the console otherwise.
@@ -120,6 +130,7 @@ class WebSocketClient {
}); });
socket.addEventListener('close', () => { socket.addEventListener('close', () => {
if (this.ws !== socket) return;
this.isConnected = false; this.isConnected = false;
this.ws = null; this.ws = null;
this.#emit('disconnected'); this.#emit('disconnected');
@@ -196,6 +207,10 @@ class WebSocketClient {
this.reconnectTimer = setTimeout(() => { this.reconnectTimer = setTimeout(() => {
this.reconnectTimer = null; this.reconnectTimer = null;
// clearTimeout doesn't cancel a callback that has already fired but
// whose macrotask hasn't run yet — re-check shouldReconnect here so
// disconnect() called in that window can't be overridden.
if (!this.shouldReconnect) return;
this.#openSocket(); this.#openSocket();
}, delay); }, delay);
} }