Skip to content

fix(web): stop flashing the reconnecting banner on self-healing blips - #1230

Merged
tiann merged 1 commit into
tiann:mainfrom
hqhq1025:fix/reconnecting-banner-debounce
Aug 1, 2026
Merged

fix(web): stop flashing the reconnecting banner on self-healing blips#1230
tiann merged 1 commit into
tiann:mainfrom
hqhq1025:fix/reconnecting-banner-debounce

Conversation

@hqhq1025

Copy link
Copy Markdown
Collaborator

Problem

EventSource fires onerror the moment a connection drops — including while it is already retrying on its own. In that case readyState is still CONNECTING and the stream is typically back within a few seconds, with nothing lost.

useSSE forwards that straight to onDisconnect (useSSE.ts:598-605), and App turned it into a full-width amber banner synchronously (App.tsx:247-253). So a blip the browser recovered from by itself still painted "正在重新连接... (流连接错误)" across the top of the screen.

The result is a banner that fires far more often than the connection is actually unusable. The sibling syncing banner already avoids exactly this, debouncing 300ms in useSyncingState.ts:38-44.

Fix

Route the disconnect through a small hook that waits out a grace period before showing anything:

  • recoveries that beat the timer stay silent
  • genuine outages still surface the banner, just a moment later — no information is lost
  • the grace period is anchored to the first drop, so repeated failed retries cannot push the banner back indefinitely

Delay is 4s, chosen to cover one full EventSource auto-reconnect cycle (~3s in Chrome) plus handshake.

Notes

No product logic changes — this only affects when an existing banner appears.

Testing

  • 6 new unit tests covering the silent-recovery path, the surfaced-outage path, reason stability across repeated drops, and timer cleanup on unmount
  • tsc --noEmit -p web/tsconfig.json clean
  • Full web suite run before and after: failure count identical (pre-existing failures unrelated to this change), passing count +6

EventSource reports an error the moment a connection drops, including
while it is already retrying by itself - readyState is still CONNECTING
and the stream is typically back within a few seconds. useSSE forwards
that straight to onDisconnect, and App turned it into a full-width
"reconnecting" banner synchronously, so a blip the browser recovered from
on its own still read as a broken network.

Route the disconnect through a small hook that waits out a grace period
first. Genuine outages still surface the banner, just a moment later;
recoveries that beat the timer stay silent. The sibling syncing banner
already debounces this way in useSyncingState.

The grace period is anchored to the first drop, so repeated failed
retries cannot push the banner back indefinitely.
@tiann
tiann force-pushed the fix/reconnecting-banner-debounce branch from 345f3c2 to cafdaa9 Compare July 29, 2026 11:57

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Keep the outage active after the grace timer fires — delayTimeoutRef is reset to null when the banner becomes visible, so a later disconnect from a replacement EventSource is treated as a new outage and can overwrite the first reason. The SSE watchdog can create that replacement without an intervening successful connect (web/src/hooks/useSSE.ts:207, web/src/hooks/useSSE.ts:617). Evidence: web/src/hooks/useReconnectingState.ts:43.
    Suggested fix:
    const outageActiveRef = useRef(false)
    
    const reportConnect = useCallback(() => {
        outageActiveRef.current = false
        clearPending()
        setIsReconnecting(false)
        setReason(null)
    }, [clearPending])
    
    const reportDisconnect = useCallback((nextReason: string) => {
        if (outageActiveRef.current) return
        outageActiveRef.current = true
        delayTimeoutRef.current = setTimeout(() => {
            delayTimeoutRef.current = null
            setIsReconnecting(true)
            setReason(nextReason)
        }, RECONNECTING_BANNER_DELAY_MS)
    }, [])

Summary
Review mode: initial

One Minor lifecycle issue found: repeated failed reconnect attempts can replace the original outage reason after the banner is already visible.

Testing

  • Not run (automation). Add a hook regression test that advances through the first delay, reports another disconnect, advances again, and asserts the original reason remains unchanged.

HAPI Bot

// A reconnect attempt can fail repeatedly; keep the grace period
// anchored to the first drop so the banner is not pushed back forever,
// and keep the reason that started the outage.
if (delayTimeoutRef.current) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MINOR] Keep the outage active after the timer fires

This guard only deduplicates while delayTimeoutRef is non-null. The timeout clears the ref when the banner appears, but the SSE watchdog can later replace the EventSource; that new instance may call onDisconnect again without any successful onConnect (web/src/hooks/useSSE.ts:207, web/src/hooks/useSSE.ts:617). A long outage that started as heartbeat-timeout/visibility-recovery can therefore schedule another timer and replace the displayed first reason with error, contrary to this hook's stated invariant.

Suggested fix:

const outageActiveRef = useRef(false)

const reportConnect = useCallback(() => {
    outageActiveRef.current = false
    clearPending()
    setIsReconnecting(false)
    setReason(null)
}, [clearPending])

const reportDisconnect = useCallback((nextReason: string) => {
    if (outageActiveRef.current) return
    outageActiveRef.current = true
    delayTimeoutRef.current = setTimeout(() => {
        delayTimeoutRef.current = null
        setIsReconnecting(true)
        setReason(nextReason)
    }, RECONNECTING_BANNER_DELAY_MS)
}, [])

Please also extend the test to call reportDisconnect again after the first delay has elapsed and assert that reason stays unchanged.

@tiann
tiann merged commit 079556b into tiann:main Aug 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants