Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The SSE lifecycle changes have no active review issues.
No open issues 🎉
What changed in this PR
This PR updates the browser SDK’s SSE lifecycle to respect native EventSource reconnection and terminal failures while preserving bounded SDK retries for custom transports.
Changes:
- Delegate native reconnection to the browser and reset custom retry budgets only after an
openevent. - Track pending and open connections separately, prevent duplicates, and ignore events from closed or replaced sources.
- Add unit and real-browser regression tests, plus a patch changeset.
| File | Description |
|---|---|
packages/browser-sdk/test/sse.test.ts |
Adds coverage for connection state, native reconnection, stale events, and custom retry limits. |
packages/browser-sdk/test/e2e/sse.browser.spec.ts |
Adds local-server browser tests for HTTP 403, reconnection, stopping, and reinitialization. |
packages/browser-sdk/src/sse.ts |
Separates native and custom retry handling and improves connection lifecycle tracking. |
.changeset/tidy-rivers-reconnect.md |
Documents the browser SDK patch release. |
Review effort: Balanced
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.
Summary
EventSourcereconnect recoverable failures without closing/recreating it in the SDK.openevent separately from an in-flight connection, avoid duplicate connections, and ignore events from closed/replaced sources.Why
connect()resolves after constructingEventSource, before the HTTP handshake completes. The previous browser retry loop reset its budget at that point, so subsequent asynchronous authentication errors never exhausted the budget.The error handler also closed every EventSource, cancelling the browser's own recoverable reconnects. The SDK interval then constructed a new source even after the browser had classified an HTTP rejection as terminal.
This change uses native retry semantics rather than adding another browser backoff/jitter loop. Native EventSource treats non-200 handshake responses as terminal, not only 403; this PR deliberately respects that classification. Explicit client stop/reinitialization remains supported.
React Native compatibility
The RN wrapper injects
react-native-ssewithpollingInterval: 0, so it depends on SDK-managed retries. An earlier revision of this PR made asynchronous custom-transport failures exhaust a finite retry budget. Review reproduced a regression: an ordinary mobile network outage lasting more than about two minutes permanently disabled live updates after connectivity returned.That policy change has been removed. Custom transports retain their existing behavior: synchronous setup failures are bounded, while asynchronous failures can continue retrying and recover after a prolonged outage. Custom-transport authentication retry policy is deliberately outside this browser-focused fix.
New tests use the actual
react-native-sseimplementation with mocked XMLHttpRequest, rather than only a generic EventSource mock. They cover starting offline and losing an established connection, three minutes offline followed by recovery, message delivery after reconnection, and cancellation on close. Both tests fail against the earlier PR revision and pass with this follow-up.react-native-sseis added only as a browser SDK dev dependency, using the same version range as the RN wrapper.Validation
yarn test: 954 tests pass across all 10 test workspaces, including 218 browser SDK tests.yarn install --immutable,yarn lint:fix,yarn fmt, andgit diff --checkpass (one existing lint warning in the management example'snext-env.d.ts).--target ES2020.Local environment limitations
yarn buildstops in the unrelated management SDK generator because Java is not installed. Its generated-file deletions were restored; no generated files are included in this PR.test/context.test.tswith the inherited pre-ES2020 target; the ES2020 override above passes.No deployment, production configuration, or credential changes.