Skip to content

Commit 3133c96

Browse files
matt423claude
andcommitted
Bound the post-open handshake wait so a silent server can't hang the spinner
If the terminal server accepted the WebSocket but never sent its hello / status:connected, the UI hung forever on the yellow "Connecting" (initial / resume) or "Reconnecting" (auto-reconnect) box. The 30s connection timeout only covers the pre-open CONNECTING phase, so once a socket opened there was nothing to bound the wait — reproduced both in the unit harness and the live dashboard. Arm an "awaiting hello" timeout (12s) when a socket opens; if it elapses before we reach `connected`, close the socket with a private recoverable code (4901) so the reconnect scheduler retries — with fresh credentials — and ultimately lands on the recoverable "Press Enter" prompt instead of a frozen spinner. The timer is cleared on connected and on close. DX-1379 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e9404f1 commit 3133c96

2 files changed

Lines changed: 177 additions & 1 deletion

File tree

packages/react-web-cli/src/AblyCliTerminal.inactivity.test.tsx

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,4 +582,130 @@ describe("DX-1379: inactivity pause & resume-on-return", () => {
582582
expect(authSend?.signature).toBe(SIGNATURE);
583583
},
584584
);
585+
586+
// The reporter's "stuck on Connecting/Reconnecting to Ably" symptom: a socket
587+
// that opens but never receives the server's hello. Before the await-hello
588+
// timeout this hung forever (the 30s connection timeout only covers the
589+
// pre-open CONNECTING phase). It must now be force-closed for retry. (DX-1379)
590+
const AWAIT_HELLO_MS = 12_000;
591+
592+
test("a socket that opens but never receives hello is force-closed for retry (not left hanging)", async () => {
593+
await act(async () => {
594+
render(
595+
<AblyCliTerminal
596+
websocketUrl={WS_URL}
597+
signedConfig={SIGNED_CONFIG}
598+
signature={SIGNATURE}
599+
resumeOnReload
600+
inactivityTimeoutMs={INACTIVITY_MS}
601+
maxReconnectAttempts={15}
602+
/>,
603+
);
604+
});
605+
await flush();
606+
const ws = sockets[0];
607+
await act(async () => {
608+
ws.fireOpen(); // opens, but the server stays silent (no hello)
609+
});
610+
await flush();
611+
612+
// Just before the threshold: still waiting, not force-closed.
613+
await act(async () => {
614+
vi.advanceTimersByTime(AWAIT_HELLO_MS - 1000);
615+
});
616+
expect(
617+
ws.close.mock.calls.some(
618+
([, reason]) => reason === "awaiting-hello-timeout",
619+
),
620+
).toBe(false);
621+
622+
// Cross the await-hello threshold -> the silent socket is closed for retry.
623+
await act(async () => {
624+
vi.advanceTimersByTime(1500);
625+
});
626+
await flush();
627+
const helloTimeoutClose = ws.close.mock.calls.find(
628+
([code, reason]) => code === 4901 && reason === "awaiting-hello-timeout",
629+
);
630+
expect(helloTimeoutClose).toBeTruthy();
631+
});
632+
633+
test("a resumed socket that never receives hello is closed for retry (the stuck-reconnecting repro)", async () => {
634+
await renderConnected("sess-hang");
635+
636+
await act(async () => {
637+
setVisibility("hidden");
638+
});
639+
await flush();
640+
await act(async () => {
641+
vi.advanceTimersByTime(INACTIVITY_MS + 10);
642+
});
643+
await flush();
644+
645+
const afterPause = sockets.length;
646+
await act(async () => {
647+
setVisibility("visible");
648+
});
649+
await flush();
650+
await act(async () => {
651+
vi.advanceTimersByTime(50);
652+
});
653+
await flush();
654+
655+
const resumeSocket = sockets[afterPause];
656+
await act(async () => {
657+
resumeSocket.fireOpen(); // resume socket opens; server never sends hello
658+
});
659+
await flush();
660+
661+
// It must not hang: after the await-hello timeout the resume socket is closed.
662+
await act(async () => {
663+
vi.advanceTimersByTime(AWAIT_HELLO_MS + 500);
664+
});
665+
await flush();
666+
const helloTimeoutClose = resumeSocket.close.mock.calls.find(
667+
([code, reason]) => code === 4901 && reason === "awaiting-hello-timeout",
668+
);
669+
expect(helloTimeoutClose).toBeTruthy();
670+
});
671+
672+
test("a hello within the await-hello window keeps the connection (no spurious close)", async () => {
673+
await act(async () => {
674+
render(
675+
<AblyCliTerminal
676+
websocketUrl={WS_URL}
677+
signedConfig={SIGNED_CONFIG}
678+
signature={SIGNATURE}
679+
resumeOnReload
680+
inactivityTimeoutMs={INACTIVITY_MS}
681+
/>,
682+
);
683+
});
684+
await flush();
685+
const ws = sockets[0];
686+
await act(async () => {
687+
ws.fireOpen();
688+
});
689+
await flush();
690+
// Server replies in time.
691+
await act(async () => {
692+
ws.fireMessage(
693+
createControlMessage({ type: "hello", sessionId: "sess-ok" }),
694+
);
695+
});
696+
await flush();
697+
698+
// Advancing past the await-hello window must NOT close the live socket.
699+
await act(async () => {
700+
vi.advanceTimersByTime(AWAIT_HELLO_MS + 2000);
701+
});
702+
await flush();
703+
// The live socket is not closed and stays usable.
704+
expect(
705+
ws.close.mock.calls.some(
706+
([, reason]) => reason === "awaiting-hello-timeout",
707+
),
708+
).toBe(false);
709+
expect(ws.readyState).toBe(FakeWebSocket.OPEN);
710+
});
585711
});

packages/react-web-cli/src/AblyCliTerminal.tsx

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,18 @@ const BASE_NON_RECOVERABLE_CLOSE_CODES = [
206206
// never confused.
207207
const INACTIVITY_PAUSE_CLOSE_CODE = 4900;
208208

209+
// Private recoverable close code used when a socket opens but the server never
210+
// sends its `hello`/`connected` within AWAIT_HELLO_TIMEOUT_MS. Closing it as a
211+
// (recoverable, not in NON_RECOVERABLE) failure lets the reconnect scheduler
212+
// retry rather than leaving the UI stuck on "Connecting"/"Reconnecting" forever.
213+
const AWAIT_HELLO_CLOSE_CODE = 4901;
214+
215+
// How long to wait, after a socket has *opened*, for the server's first
216+
// `hello`/`status: connected` before treating the attempt as failed. The 30s
217+
// connection timeout only covers the pre-open CONNECTING phase, so without this
218+
// a silent-but-accepting server hangs the spinner indefinitely (DX-1379).
219+
const AWAIT_HELLO_TIMEOUT_MS = 12_000;
220+
209221
const AblyCliTerminalInner = (
210222
{
211223
websocketUrl,
@@ -685,9 +697,14 @@ const AblyCliTerminalInner = (
685697
(status: ConnectionStatus) => {
686698
// updateConnectionStatusAndExpose debug removed
687699
// A successful connection clears any in-flight inactivity-resume guard so
688-
// a later unrelated disconnect isn't misread as a failed resume.
700+
// a later unrelated disconnect isn't misread as a failed resume, and the
701+
// "awaiting hello" timeout (the handshake completed).
689702
if (status === "connected") {
690703
resumeAttemptReference.current = false;
704+
if (awaitHelloTimerReference.current) {
705+
clearTimeout(awaitHelloTimerReference.current);
706+
awaitHelloTimerReference.current = null;
707+
}
691708
}
692709
setComponentConnectionStatusState(status);
693710
// (window as any).componentConnectionStatusForTest = status; // Keep for direct inspection if needed, but primary is below
@@ -761,6 +778,17 @@ const AblyCliTerminalInner = (
761778
}
762779
}, []);
763780

781+
// "Awaiting hello" timeout: armed once a socket opens, cleared on
782+
// connected/close. Guards against a server that accepts the socket but never
783+
// completes the handshake.
784+
const awaitHelloTimerReference = useRef<NodeJS.Timeout | null>(null);
785+
const clearAwaitHelloTimer = useCallback(() => {
786+
if (awaitHelloTimerReference.current) {
787+
clearTimeout(awaitHelloTimerReference.current);
788+
awaitHelloTimerReference.current = null;
789+
}
790+
}, []);
791+
764792
const clearInstallInstructionsTimer = useCallback(() => {
765793
if (installInstructionsTimerReference.current) {
766794
clearTimeout(installInstructionsTimerReference.current);
@@ -1577,6 +1605,24 @@ const AblyCliTerminalInner = (
15771605
sock.send(JSON.stringify(payload));
15781606
}
15791607

1608+
// Arm the "awaiting hello" timeout: the socket is open, but until the
1609+
// server sends its hello/connected we have no session. If it never
1610+
// arrives, force-close this socket as a recoverable failure so the
1611+
// reconnect scheduler retries instead of the UI hanging on the spinner.
1612+
clearAwaitHelloTimer();
1613+
awaitHelloTimerReference.current = setTimeout(() => {
1614+
if (connectionStatusReference.current !== "connected") {
1615+
debugLog(
1616+
`⚠️ DIAGNOSTIC: No hello within ${AWAIT_HELLO_TIMEOUT_MS}ms of open - closing socket to retry`,
1617+
);
1618+
try {
1619+
sock.close(AWAIT_HELLO_CLOSE_CODE, "awaiting-hello-timeout");
1620+
} catch {
1621+
/* ignore */
1622+
}
1623+
}
1624+
}, AWAIT_HELLO_TIMEOUT_MS);
1625+
15801626
// Set up initial command to be sent when prompt is detected
15811627
// Skip initial command if we're resuming an existing session
15821628
if (initialCommand && !sessionId) {
@@ -1600,6 +1646,7 @@ const AblyCliTerminalInner = (
16001646
clearPtyBuffer,
16011647
sessionId,
16021648
clearConnectionTimeout,
1649+
clearAwaitHelloTimer,
16031650
refreshAuth,
16041651
],
16051652
);
@@ -1844,6 +1891,7 @@ const AblyCliTerminalInner = (
18441891
`[AblyCLITerminal] WebSocket closed. Code: ${event.code}, Reason: ${event.reason}, Clean: ${event.wasClean}`,
18451892
);
18461893
clearConnectionTimeout(); // Clear timeout on close
1894+
clearAwaitHelloTimer();
18471895
clearTerminalBoxOnly();
18481896
updateSessionActive(false);
18491897

@@ -2070,6 +2118,7 @@ const AblyCliTerminalInner = (
20702118
resumeOnReload,
20712119
sessionId,
20722120
clearConnectionTimeout,
2121+
clearAwaitHelloTimer,
20732122
clearPtyBuffer,
20742123
],
20752124
);
@@ -2796,6 +2845,7 @@ const AblyCliTerminalInner = (
27962845
]);
27972846

27982847
useEffect(() => () => clearInactivityTimer(), [clearInactivityTimer]);
2848+
useEffect(() => () => clearAwaitHelloTimer(), [clearAwaitHelloTimer]);
27992849

28002850
// Cleanup install instructions timer on unmount
28012851
useEffect(

0 commit comments

Comments
 (0)