fix: reconnect banner never cleared after SSE reconnect#129
Merged
Conversation
The reconnect manager cleared the "Reconnecting…" banner (and reset data-via-connection to "online") only on datastar-fetch 'started'/'finished'. But a long-lived SSE @get stream fires 'retrying' when it drops and emits NO 'started'/'finished' when it successfully resumes — so after a real reconnect the banner stayed stuck showing "Reconnecting…" and the connection attribute stayed "connecting" forever, even though data was flowing again. Clear on incoming SSE patches (datastar-patch-elements / datastar-patch-signals) instead: an arrived patch — the reconnect re-bootstrap, or via's periodic heartbeat — is the only reliable "stream is alive again" signal for a streaming fetch. Found by killing the server under a live browser; the prior browser test dispatched synthetic started/finished events, so it never exercised the real reconnect path. The test now drives a patch event and asserts the banner clears.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second bug found by the in-browser verification campaign (and worse than the first).
The reconnect manager (#122/#126) cleared the "Reconnecting…" banner / reset
data-via-connectiontoonlineonly ondatastar-fetchstarted/finished. But a long-lived SSE@getstream fires onlyretryingwhen it drops — no started/finished on a successful resume. So after a real reconnect the banner stayed stuck "Reconnecting…" and the attribute stuck "connecting" forever, even though patches were flowing again. (Verified live: killed the server, watched it reconnect —localre-bootstrapped to fresh state while the banner never cleared.)Fix
Clear on incoming SSE patch events (
datastar-patch-elements/datastar-patch-signals) — an arrived patch (the reconnect re-bootstrap, or via's 25s heartbeat) is the only reliable "stream alive again" signal for a streaming fetch.Why it escaped the chromedp test
The browser test dispatched synthetic started/finished events, so it exercised a path that doesn't occur on a real reconnect. The test now dispatches a patch event and asserts the banner clears + connection returns to online (online→connecting→online→offline lifecycle).
Full
ci-check.shgreen; browser test passes locally.