re-land the client half: hold the socket across a remount, and queue what was typed
Three commits backed out a few hours ago as collateral, restored together because they are one fix:243bd04(queue sends until OPEN),bc13450(defer the teardown close by a tick, cancellable) and66a41d0(count CONNECTING as ours, not only OPEN). Why they are needed, from evidence rather than reasoning.d9857eefixed the server side and Andre still had nothing on https://macbook.pastilhas.dev after a restart. The decisive observation is an ABSENCE: his attempts appear nowhere in officer's log — no "Model selected for chat", no claude:stream for his session. Nothing reaches the server at all. Meanwhile a socket I drove by hand against the same wss:// url ran a full turn in 2.5s, so the transport is not it. That is `send` dropping the message. It returned silently on `readyState !== OPEN`, and the socket is not OPEN because the effect cleanup closed it on a remount while it was still CONNECTING, then closed its replacement the same way. Enter does nothing, forever, with the view sitting on Disconnected — and no error anywhere, on either side, which is why this reads as a dead server. Note what is still NOT fixed, and is written into the code comment rather than this message alone: a /chat/new load opens TWO sockets, from two separate useChat instances mounting. Measured with a constructor counter. Both connect, so it looks healthy;d9857eeis what makes it harmless. Correction to d9857ee's message, so the record is not wrong: it claims the localhost/domain split is latency changing the attach/close ordering. That is plausible and it is NOT what was demonstrated — the server-side fix alone did not help. It is still worth having (a stale close silencing a live client is real, and so is the idle GC gate), but the asymmetry is unexplained and the client drop above is what actually stopped a message. Client bundle changes, so this needs a hard reload as well as a restart. Not verified in a browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -15,14 +15,39 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar
|
|||||||
const retryRef = useRef(0);
|
const retryRef = useRef(0);
|
||||||
const retryTimeoutRef = useRef<number | null>(null);
|
const retryTimeoutRef = useRef<number | null>(null);
|
||||||
const isCleaningUpRef = useRef(false);
|
const isCleaningUpRef = useRef(false);
|
||||||
|
/** A deferred teardown, cancelled when the effect re-runs — see the cleanup below. */
|
||||||
|
const closeTimerRef = useRef<number | null>(null);
|
||||||
const onMessageRef = useRef(onMessage);
|
const onMessageRef = useRef(onMessage);
|
||||||
onMessageRef.current = onMessage;
|
onMessageRef.current = onMessage;
|
||||||
const onOpenRef = useRef(onOpen);
|
const onOpenRef = useRef(onOpen);
|
||||||
onOpenRef.current = onOpen;
|
onOpenRef.current = onOpen;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Messages typed before the socket was ready.
|
||||||
|
*
|
||||||
|
* `send` used to drop them: `readyState !== OPEN` returned, silently, with no error and no retry — so
|
||||||
|
* pressing enter did nothing and the turn never happened. That window is not rare. React's dev
|
||||||
|
* StrictMode double-invokes effects, so every socket is created, closed and recreated on mount, and a
|
||||||
|
* reconnect after a drop reopens it again; with several chat panes on screen there are several sockets
|
||||||
|
* doing this at once. One of them is always briefly not OPEN.
|
||||||
|
*
|
||||||
|
* Queued and flushed on open, in order. The mobile chat app does exactly this and for exactly this
|
||||||
|
* reason — the composer is allowed to fire before the transport is ready.
|
||||||
|
*/
|
||||||
|
const pendingRef = useRef<string[]>([]);
|
||||||
|
|
||||||
const connect = () => {
|
const connect = () => {
|
||||||
if (isCleaningUpRef.current) return;
|
if (isCleaningUpRef.current) return;
|
||||||
if (socketRef.current && socketRef.current.readyState === WebSocket.OPEN) return;
|
// CONNECTING counts as ours, not just OPEN. The deferred teardown below keeps a remount's socket
|
||||||
|
// alive mid-handshake, and this is what reclaims it: checking only OPEN meant an effect re-run
|
||||||
|
// built a SECOND socket and orphaned the first, which then stayed open forever with its own `open`
|
||||||
|
// handler bailing on the socketRef mismatch.
|
||||||
|
//
|
||||||
|
// This is per-instance and does NOT explain the two sockets a /chat/new load opens — measured with
|
||||||
|
// a WebSocket-constructor counter, those come from two separate `useChat` instances mounting, each
|
||||||
|
// with its own refs. Unresolved, and tracked separately; both connect, so it reads as healthy.
|
||||||
|
const existing = socketRef.current;
|
||||||
|
if (existing && (existing.readyState === WebSocket.OPEN || existing.readyState === WebSocket.CONNECTING)) return;
|
||||||
|
|
||||||
const socket = new WebSocket(url);
|
const socket = new WebSocket(url);
|
||||||
socketRef.current = socket;
|
socketRef.current = socket;
|
||||||
@@ -31,7 +56,12 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar
|
|||||||
if (socketRef.current !== socket) return;
|
if (socketRef.current !== socket) return;
|
||||||
setIsConnected(true);
|
setIsConnected(true);
|
||||||
retryRef.current = 0;
|
retryRef.current = 0;
|
||||||
|
// BEFORE onOpen, deliberately: onOpen sends the resume/attach handshake, and anything the user
|
||||||
|
// typed while connecting belongs after that, not in front of it.
|
||||||
|
const queued = pendingRef.current;
|
||||||
|
pendingRef.current = [];
|
||||||
onOpenRef.current?.();
|
onOpenRef.current?.();
|
||||||
|
for (const message of queued) socket.send(message);
|
||||||
});
|
});
|
||||||
|
|
||||||
socket.addEventListener('message', (ev) => {
|
socket.addEventListener('message', (ev) => {
|
||||||
@@ -59,25 +89,56 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar
|
|||||||
};
|
};
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
// A pending teardown from a remount that is about to be undone — see below.
|
||||||
|
if (closeTimerRef.current !== null) {
|
||||||
|
clearTimeout(closeTimerRef.current);
|
||||||
|
closeTimerRef.current = null;
|
||||||
|
}
|
||||||
isCleaningUpRef.current = false;
|
isCleaningUpRef.current = false;
|
||||||
connect();
|
connect();
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
|
/**
|
||||||
|
* Close LATER, not now.
|
||||||
|
*
|
||||||
|
* Closing here directly is correct for a real unmount and disastrous for a remount, and this hook
|
||||||
|
* cannot tell them apart at the moment it runs. React's dev StrictMode double-invokes every effect
|
||||||
|
* (mount → unmount → mount), and a subtree that is re-created — a resolved transcript, a parent key
|
||||||
|
* change — does the same. Each time, the socket was closed while still CONNECTING, the browser
|
||||||
|
* logged "closed before the connection is established", and the replacement was closed in turn, so
|
||||||
|
* the view could churn forever and never hold a connection while sitting on Disconnected.
|
||||||
|
*
|
||||||
|
* Deferring by a tick makes the two distinguishable. A remount re-runs the effect immediately and
|
||||||
|
* cancels this timer, so the live socket is kept and the handshake completes. A real unmount has
|
||||||
|
* nobody to cancel it and the socket closes a frame later, which costs nothing.
|
||||||
|
*/
|
||||||
isCleaningUpRef.current = true;
|
isCleaningUpRef.current = true;
|
||||||
if (retryTimeoutRef.current !== null) {
|
if (retryTimeoutRef.current !== null) {
|
||||||
clearTimeout(retryTimeoutRef.current);
|
clearTimeout(retryTimeoutRef.current);
|
||||||
retryTimeoutRef.current = null;
|
retryTimeoutRef.current = null;
|
||||||
}
|
}
|
||||||
if (socketRef.current) {
|
const socket = socketRef.current;
|
||||||
socketRef.current.close();
|
closeTimerRef.current = window.setTimeout(() => {
|
||||||
socketRef.current = null;
|
closeTimerRef.current = null;
|
||||||
}
|
if (!isCleaningUpRef.current) return; // remounted: the effect above already reclaimed it
|
||||||
|
if (socket) socket.close();
|
||||||
|
if (socketRef.current === socket) socketRef.current = null;
|
||||||
|
}, 0);
|
||||||
};
|
};
|
||||||
}, [url]);
|
}, [url]);
|
||||||
|
|
||||||
const send = (data: Record<string, unknown>) => {
|
const send = (data: Record<string, unknown>) => {
|
||||||
const socket = socketRef.current;
|
const socket = socketRef.current;
|
||||||
if (!socket || socket.readyState !== WebSocket.OPEN) return;
|
const message = JSON.stringify(data);
|
||||||
socket.send(JSON.stringify(data));
|
if (socket && socket.readyState === WebSocket.OPEN) {
|
||||||
|
socket.send(message);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Not open yet, or reconnecting. Hold it rather than dropping it — see `pendingRef`. Bounded so a
|
||||||
|
// socket that never comes back cannot grow this without limit; the oldest go first, because the
|
||||||
|
// newest message is the one the user is still waiting on.
|
||||||
|
pendingRef.current.push(message);
|
||||||
|
if (pendingRef.current.length > 50) pendingRef.current.shift();
|
||||||
};
|
};
|
||||||
|
|
||||||
return { isConnected, send };
|
return { isConnected, send };
|
||||||
|
|||||||
Reference in New Issue
Block a user