Private
Public Access
Fix stale-socket race in WS reconnect logic
React StrictMode double-invokes the effect that opens the chat WebSocket in dev (mount -> cleanup -> mount again, on purpose, to catch exactly this class of bug). The first socket gets abandoned in cleanup, but its onclose still fires asynchronously afterward -- and unconditionally ran `socketRef.current = null`, even after the second (real) socket had already taken over. That silently orphaned a perfectly live connection: still open and receiving broadcasts fine, but nothing left holding a reference to send on, so outgoing messages/edits went nowhere with no visible error. Found via manual testing: messages sent through the UI weren't appearing, but a raw WebSocket opened by hand (bypassing React entirely) joined and sent a message successfully, isolating the bug to the reconnect logic rather than the backend. Fix: each socket's onopen/onclose now checks it's still the one referenced by socketRef before mutating shared state, so a stale/superseded socket's events are inert instead of clobbering whatever socket is actually current. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -29,16 +29,30 @@ export function useChatSocket({ roomId, onMessage, onUnauthenticated }: UseChatS
|
||||
socketRef.current = ws
|
||||
|
||||
ws.onopen = () => {
|
||||
// Guards against React StrictMode's dev-only double-invoke of this
|
||||
// effect (mount -> cleanup -> mount again): the first socket gets
|
||||
// abandoned in cleanup, but its own open/close events can still
|
||||
// fire asynchronously afterward. Without this check, a stale
|
||||
// socket's callbacks can stomp on state that the second (real)
|
||||
// socket already owns.
|
||||
if (socketRef.current !== ws) return
|
||||
reconnectDelay = RECONNECT_BASE_DELAY_MS
|
||||
setConnected(true)
|
||||
ws.send(JSON.stringify({ type: 'join', room_id: roomId }))
|
||||
}
|
||||
|
||||
ws.onmessage = (event) => {
|
||||
if (socketRef.current !== ws) return
|
||||
onMessageRef.current(JSON.parse(event.data) as ServerEnvelope)
|
||||
}
|
||||
|
||||
ws.onclose = (event) => {
|
||||
// Same guard as onopen -- a stale/abandoned socket's close event
|
||||
// must not null out the reference to whatever socket has actually
|
||||
// taken over since (this was a real bug: the abandoned socket's
|
||||
// delayed onclose was silently orphaning a perfectly live
|
||||
// connection, with nothing left referencing it to send on).
|
||||
if (socketRef.current !== ws) return
|
||||
setConnected(false)
|
||||
socketRef.current = null
|
||||
|
||||
|
||||
Reference in New Issue
Block a user