fix(sse): honor disconnect/dispose and cap error-path reconnects#2277
Merged
Conversation
SSE auto-reconnect ignored disconnectSSE and screen dispose because _manuallyDisconnected was never checked before scheduling reconnects, and dispose() cleared that set before delayed callbacks could run. The error handler also passed reconnectAttempts by value, so the outer counter never advanced and maxReconnectAttempts was bypassed on errors. Guard reconnect with _shouldReconnect (dispose + manual disconnect) and share attempt state via a list so error-driven retries respect the cap. Co-authored-by: Sharjeel Yunus <sharjeelyunus@users.noreply.github.com>
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
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.
Bug and impact
Trigger: A screen subscribes to an SSE API (
invokeAPIwithlistenForChanges/ SSE provider). The user callsdisconnectSSE, navigates away (screen dispose), or the endpoint errors repeatedly.Impact:
disconnect()anddispose()set_manuallyDisconnectedbut reconnect logic never checked it.dispose()also cleared the set immediately, so delayedonDonecallbacks still scheduled reconnects against closed HTTP clients — background network activity, battery drain, and possible duplicate listeners._handleSSEErrorreceivedreconnectAttemptsby value; increments inside the handler did not update the outer counter in_connectSSE. WithautoReconnect: true(default), every stream error retried withattempt == 0, bypassingmaxReconnectAttempts(default 5) and causing an infinite reconnect loop on persistent failures.Root cause
_manuallyDisconnectedwas write-only (set indisconnect/dispose, never read inonDone/_handleSSEError).dispose()cleared_manuallyDisconnectedafter cancelling subscriptions.intinstead of shared mutable attempt state.Fix
_disposedflag and central_shouldReconnect()guard (checks dispose, manual disconnect,autoReconnect, andmaxReconnectAttempts).connect().List<int>so error andonDonepaths increment the same counter._manuallyDisconnectedindispose().Validation
modules/ensemble/test/sse_provider_dispose_test.dartwith reconnect guard tests (disconnect, dispose, max attempts, shared counter).modules/ensemble:flutter test test/sse_provider_dispose_test.dartDuplicate check
cursor/critical-bug-remediation-d916): scoped SSE dispose to per-screen provider instances — related but does not fix reconnect-after-disconnect or error-path attempt bypass.cursor/critical-bug-remediation-76ab: CDN_lastUpdatedAtpremature commit — different bug, not re-opened here.maxReconnectAttemptsbypass.