Skip to content

Commit ec1bcde

Browse files
fix(web): connection health poll silently erased the escalated "down" state
useLiveStream's health-poll effect (every 10s) unconditionally reset attemptsRef to 0 and overwrote `connection` on every successful GET /api/health — regardless of whether the actual EventSource was open. Since a plain health check answering says nothing about the SSE stream itself, this meant that a few seconds after the UI escalated to "down" (after 3 consecutive stream failures), the next poll tick would silently flip it back to "reconnecting" and reset the reconnect backoff to its fastest delay — so a user watching the connection badge would almost never actually see "down", losing the signal exactly when the stream had been failing repeatedly. Also fixed connect() itself, which set connection to 'reconnecting' on every dispatched retry attempt without checking whether attempts had already crossed the DOWN_AFTER_ATTEMPTS threshold — the same downgrade bug via a different path. Now the poll only promotes state (and resets backoff) when the EventSource is genuinely OPEN; otherwise it leaves the EventSource's own onerror/onopen handlers as the sole source of truth for connection state. Caught by web/src/hooks/useLiveStream.test.tsx ("escalates to down after repeated failures and resumes from the last seq"), which was failing on main before this change.
1 parent 6e1efe9 commit ec1bcde

1 file changed

Lines changed: 24 additions & 4 deletions

File tree

web/src/hooks/useLiveStream.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,18 @@ export function useLiveStream(opts: StreamOptions = {}): LiveStream {
160160
const es = createEventSource(lastSeqRef.current);
161161
esRef.current = es;
162162
if (mountedRef.current) {
163-
setConnection(attemptsRef.current > 0 ? 'reconnecting' : 'connecting');
163+
// Once escalated to 'down', a dispatched retry must not silently
164+
// downgrade the banner back to 'reconnecting' — the user would lose
165+
// the severity signal on every subsequent attempt, right when it
166+
// matters most. 'down' only clears on an actual successful connection
167+
// (the 'connected'/onopen handlers reset attemptsRef to 0 below).
168+
setConnection(
169+
attemptsRef.current >= DOWN_AFTER_ATTEMPTS
170+
? 'down'
171+
: attemptsRef.current > 0
172+
? 'reconnecting'
173+
: 'connecting',
174+
);
164175
}
165176

166177
es.addEventListener('connected', () => {
@@ -314,10 +325,19 @@ export function useLiveStream(opts: StreamOptions = {}): LiveStream {
314325
try {
315326
const h = await getHealth();
316327
if (cancelled || !mountedRef.current) return;
317-
attemptsRef.current = 0;
318328
const es = esRef.current;
319-
// EventSource.OPEN === 1; CONNECTING === 0.
320-
setConnection(es && es.readyState === 1 ? 'live' : 'reconnecting');
329+
// EventSource.OPEN === 1; CONNECTING === 0. A successful health
330+
// check only means the plain HTTP API answered — it says nothing
331+
// about the SSE connection itself. Only promote state (and reset
332+
// the backoff) when the EventSource is genuinely open; otherwise
333+
// leave connection/attemptsRef alone so the escalated 'down' state
334+
// from repeated onerror failures survives until the stream
335+
// actually reconnects, instead of flapping back to 'reconnecting'
336+
// (and resetting backoff to its fastest delay) on every poll tick.
337+
if (es && es.readyState === 1) {
338+
attemptsRef.current = 0;
339+
setConnection('live');
340+
}
321341
if (typeof h.running === 'boolean' && h.running !== runningRef.current) {
322342
setRunning(h.running);
323343
}

0 commit comments

Comments
 (0)