feat(analytics-core): retain events and go offline after max retries on client SDKs - #1908
feat(analytics-core): retain events and go offline after max retries on client SDKs#1908Mercy811 wants to merge 3 commits into
Conversation
size-limit report 📦
|
4a720cd to
5362fdc
Compare
Mercy811
changed the title
309a934 to
d510e31
Compare
…on client SDKs When the event server is unreachable (5xx / network errors / timeouts), the Browser and React Native SDKs dropped events once the retry budget was exhausted. Offline mode is only entered on a device-network signal (navigator.onLine / NetInfo), so a server-only outage never trips it. During the 2026-07-24 AWS us-west-2 outage this dropped ~0.4% of web events. Mirror the mobile SDKs (Amplitude-Swift EventPipeline), for client SDKs only: - Retry transient failures with exponential backoff (retryTimeout * 2^(attempts-1)) instead of linear. The retry count (flushMaxRetries, default 5 on browser/RN) bounds it, so no interval cap is needed. - Once the retry budget is exhausted, set config.offline = true and retain the events instead of dropping them. They stay in the queue and on storage; flushing resumes when the connectivity checker sees a navigator/NetInfo online event or the page reloads / app relaunches (which resets config.offline and, since context.attempts is not persisted, retries every stored event from scratch). No server polling. Gate this on the environment via a new isClientSide() helper (isReactNative() || isBrowser()). The Node (server) SDK is a long-lived process with no reconnect/reload recovery, so it keeps the legacy behavior of dropping events past the retry budget. Terminal per-event drops are unchanged (400 invalid API key, server-flagged events, 413 single-event). Update tests for the new client behavior: drop the browser exhaust-retries drop assertion, and stabilize RN init by disabling attribution where it caused flakes. Refs SDKW-42, SDKRN-55. Co-authored-by: Cursor <cursoragent@cursor.com>
d510e31 to
03fc4a8
Compare
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5597e11. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5597e116b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
isClientSide() used `document` as the browser discriminator, so a document-less browser context — notably a Manifest V3 extension background service worker, as in examples/browser/chrome-ext — fell through to the Node branch and dropped events past the retry budget instead of retaining them. Add isWebWorker(), which confirms the global scope is an instance of its own WorkerGlobalScope. That constructor is exposed only on worker globals, and the instanceof check additionally rules out non-browser runtimes that expose web constructors on a server global. OR it into isClientSide() along with the existing isChromeExtension(). Keep isBrowser/isClientSide/isWebWorker out of the public API — nothing outside analytics-core uses them. Refs SDKW-42, SDKRN-55. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| isExceedingMaxRetries = true; | ||
| } | ||
| }); | ||
| if (isExceedingMaxRetries) { |
There was a problem hiding this comment.
By my spreadsheet calculations, it would (by default) take ~34 minutes before it stops trying and then goes "offline".
To confirm, we still save events to offline storage (storageProvider) before they go offline? It's just that marking the client as "offline" prevents the unsent events from being flushed whenever there's a failure?
There was a problem hiding this comment.
could you share your calculation, I get 16s from event tracked to offline with exponential backoff:
- track 1s flush, 1s
- failure 1, 2^0 = 1s
- failure 2, 2s
- failure 3, 4s
- failure 4, 8s
- failure 5, offline
total = 1 + 1 + 2 + 4 + 8 = 16s
There was a problem hiding this comment.
To confirm, we still save events to offline storage (storageProvider) before they go offline? It's just that marking the client as "offline" prevents the unsent events from being flushed whenever there's a failure?
Yes, saveEvents() is called every time on destination.execute()
There was a problem hiding this comment.
Isn't it 12 retries? When you count up to 12 that's how I calculate that.
There was a problem hiding this comment.
that's probably for node, browser default is at
79503fe to
ba2a055
Compare
|
|
||
| test('should exhaust max retries', async () => { | ||
| const scope = nock(url).post(path).times(3).reply(500, { | ||
| code: 500, |
There was a problem hiding this comment.
it looks like every response that goes through handleOtherResponse gets the new offline logic - are there any responses that we should not retry?
There was a problem hiding this comment.
good catch!
- before: the logic is always retry except knowing to drop (invalid, missing fields etc. identified by server) and then drop.
- after: the logic is retry and then offline so a bad event not identified by server can cause the SDK offline forever. But it seems that this logic works well for mobile SDKs. So we could safely take the assumption that server will tell SDKs which events to drop and the should retry rest events
We shouldn't retry non-tryable 5xx for example #1630. Thanks @daniel-graham-amplitude for bring this up.
3 participants
Summary
Follow-up to the 2026-07-24 AWS us-west-2 event-server outage (SDKW-42, SDKRN-55).
When the event server is unreachable (5xx / network errors / timeouts), the Browser and React Native SDKs dropped events once the retry budget was exhausted. Offline mode is only entered on a device-network signal (
navigator.onLine/ NetInfo), so a server-only outage — where the device network stays up — never trips it and events are dropped after retries.This change makes
analytics-core'sDestinationmirror the mobile SDKs (Amplitude-SwiftEventPipeline) — for client SDKs only.What changes (
packages/analytics-core/src/plugins/destination.ts)retryTimeout * 2^(attempts-1)(was linearattempts * retryTimeout). No interval cap: the retry count (flushMaxRetries, default 5 on browser/RN) already bounds it (largest wait ~8s before going offline at defaults).flushMaxRetrieson a transient failure, setconfig.offline = trueand keep the event (it stays in the queue + storage) rather than dropping it viaremoveEventsExceedFlushMaxRetries.Client vs. server gating (important)
The
Destinationplugin lives inanalytics-coreand is shared by the Node (server) SDK too. Node is a long-lived process with no reload/relaunch and no connectivity checker, so flipping it offline would strand it forever. The go-offline behavior is therefore gated to client SDKs only, via a newisClientSide()helper (isReactNative() || isBrowser(), alongside the existingisReactNative()inutils/environment.ts), resolved once as a field initializer (the runtime environment is fixed for the process lifetime):Recovery (client SDKs — mobile parity)
Deliberately no server polling. Client SDKs come back online exactly like mobile:
config.offline = falseon anavigator/ NetInfo online event, orconfig.offlineis in-memory so it resets, and becausecontext.attemptsis not persisted, every stored event is retried from a fresh budget.Events are retained across the outage and delivered on recovery — only storage exhaustion would lose them.
Scope / behavior notes
handleOtherResponse— 5xx / network / timeout) changes. Terminal per-event drops are untouched: 400 invalid API key, server-flagged invalid events, and 413 single-event still drop viaremoveEventsExceedFlushMaxRetries.OfflineDisabledis not special-cased in the destination (it only gates installing the connectivity plugin,browser-client.ts:312); the destination just reads/writesconfig.offline. On a client SDK that setsoffline: OfflineDisabled, a max-retry still flips it totrueand retains — same as mobile.Open question
Refs SDKW-42, SDKRN-55.
🤖 Generated with Claude Code
Note
Medium Risk
Changes core event upload retry/drop behavior for all browser and React Native customers during outages; Node path is explicitly preserved but shared Destination code is security-sensitive for data loss.
Overview
Client SDKs (browser / React Native) no longer drop events when transient upload failures exhaust
flushMaxRetries.Destination.handleOtherResponsenow uses exponential backoff (getRetryBackoff), and on the last failed attempt setsconfig.offline = true, keeps events in the queue/storage, and recordsoffline.by.max.retries—pausing flush until connectivity recovery or reload (no server polling).Node keeps prior behavior: linear backoff and drop via
removeEventsExceedFlushMaxRetries. Branching uses newisBrowser/isClientSidehelpers (exported fromanalytics-core).Tests cover the new retry/offline paths; the browser integration test that expected max-retry rejection was removed. RN tests disable attribution where needed for stability.
Reviewed by Cursor Bugbot for commit 5597e11. Configure here.