Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions packages/analytics-browser-test/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -800,47 +800,6 @@ describe('integration', () => {
second.done();
});

test('should exhaust max retries', async () => {
const scope = nock(url).post(path).times(3).reply(500, {
code: 500,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it looks like every response that goes through handleOtherResponse gets the new offline logic - are there any responses that we should not retry?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

});

await client.init(apiKey, {
logLevel: 0,
flushMaxRetries: 3,
defaultTracking,
fetchRemoteConfig: false,
}).promise;
const response = await client.track('test event').promise;
expect(response.event).toEqual({
user_id: undefined,
device_id: uuid,
session_id: number,
time: number,
platform: 'Web',
language: 'en-US',
ip: '$remote',
insert_id: uuid,
partner_id: undefined,
event_type: 'test event',
event_properties: {
'[Amplitude] Page Domain': '',
'[Amplitude] Page Location': '',
'[Amplitude] Page Path': '',
'[Amplitude] Page Title': '',
'[Amplitude] Page URL': '',
'[Amplitude] Previous Page Location': '',
'[Amplitude] Previous Page Type': 'direct',
},
event_id: 0,
library: library,
user_agent: userAgent,
});
expect(response.code).toBe(500);
expect(response.message).toBe('Event rejected due to exceeded retry count');
scope.done();
}, 10000);

test('should handle missing api key', async () => {
await client.init('', undefined, {
logLevel: 0,
Expand Down
52 changes: 47 additions & 5 deletions packages/analytics-core/src/plugins/destination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { EventCallback } from '../types/event-callback';
import { IDiagnosticsClient } from '../diagnostics/diagnostics-client';
import { isSuccessStatusCode } from '../utils/status-code';
import { getStacktrace } from '../utils/debug';
import { isClientSide } from '../utils/environment';

export interface Context {
event: Event;
Expand Down Expand Up @@ -95,6 +96,11 @@ export class Destination implements DestinationPlugin {
flushId: ReturnType<typeof setTimeout> | null = null;
queue: Context[] = [];
diagnosticsClient: IDiagnosticsClient | undefined;
// True for client SDKs (browser page or worker, Chrome extension, React Native), which can
// recover from an offline state via a network reconnect or a page reload / worker restart /
// app relaunch. False for the Node (server) SDK — a long-lived process with no such
// recovery — where events past the retry budget are dropped as before.
isClientSide = isClientSide();

constructor(context?: { diagnosticsClient: IDiagnosticsClient }) {
this.diagnosticsClient = context?.diagnosticsClient;
Expand Down Expand Up @@ -390,13 +396,49 @@ export class Destination implements DestinationPlugin {
this.scheduleEvents(tryable);
}

getRetryBackoff(attempts: number): number {
return this.retryTimeout * Math.pow(2, Math.max(0, attempts - 1));
}

handleOtherResponse(list: Context[]) {
const later = list.map((context) => {
context.timeout = context.attempts * this.retryTimeout;
return context;
});
let tryable: Context[];

if (this.isClientSide) {
// Client SDKs: mirror the mobile SDKs. Retry with exponential backoff, and once the
// retry budget is exhausted, go offline and keep the events instead of dropping
// them — this pauses all further flushing. They stay in the queue + storage; flushing
// resumes when the connectivity checker sees a `navigator`/NetInfo online event, or on
// page reload / worker restart / app relaunch (which resets config.offline and, since
// `attempts` is not persisted, retries from scratch).
tryable = [];
let isExceedingMaxRetries = false;
list.forEach((context) => {
context.attempts += 1;
if (context.attempts < this.config.flushMaxRetries) {
context.timeout = this.getRetryBackoff(context.attempts);
tryable.push(context);
} else {
isExceedingMaxRetries = true;
}
});
if (isExceedingMaxRetries) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

could you share your calculation, I get 16s from event tracked to offline with exponential backoff:

  1. track 1s flush, 1s
  2. failure 1, 2^0 = 1s
  3. failure 2, 2s
  4. failure 3, 4s
  5. failure 4, 8s
  6. failure 5, offline
    total = 1 + 1 + 2 + 4 + 8 = 16s

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't it 12 retries? When you count up to 12 that's how I calculate that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that's probably for node, browser default is at

public flushMaxRetries: number = 5,

this.config.offline = true;
this.config.loggerProvider.debug(
`Upload failed after ${this.config.flushMaxRetries} retries; going offline and pausing flush until the SDK reconnects or the page reloads.`,
);
this.diagnosticsClient?.increment('offline.by.max.retries');
}
} else {
// Server (Node) SDK: a long-lived process with no reconnect/reload to recover from
// offline, so keep the legacy behavior — linear backoff and drop events past the
// retry budget.
const later = list.map((context) => {
context.timeout = context.attempts * this.retryTimeout;
return context;
});
tryable = this.removeEventsExceedFlushMaxRetries(later);
}

const tryable = this.removeEventsExceedFlushMaxRetries(later);
this.scheduleEvents(tryable);
}

Expand Down
23 changes: 23 additions & 0 deletions packages/analytics-core/src/utils/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,26 @@ export function isReactNative(): boolean {
const globalScope = getGlobalScope() as { navigator?: { product?: string } };
return globalScope?.navigator?.product === 'ReactNative';
}

// Main-thread browser only. Workers have no `document` — use isWebWorker() for those.
export function isBrowser(): boolean {
const globalScope = getGlobalScope() as { document?: unknown } | undefined;
return typeof globalScope?.document !== 'undefined';
Comment thread
Mercy811 marked this conversation as resolved.
}

// True inside a dedicated / shared / service worker, e.g. a Manifest V3 extension background
// worker. `WorkerGlobalScope` is only exposed on worker globals, so its presence is already
// worker-specific; the instanceof check additionally rules out non-browser runtimes that expose
// web constructors on a server global.
export function isWebWorker(): boolean {
const globalScope = getGlobalScope() as { WorkerGlobalScope?: new () => unknown } | undefined;
if (!globalScope) {
return false;
}
const workerGlobalScope = globalScope.WorkerGlobalScope;
return typeof workerGlobalScope === 'function' && globalScope instanceof workerGlobalScope;
}

export function isClientSide(): boolean {
return isReactNative() || isBrowser() || isWebWorker() || isChromeExtension();
}
98 changes: 98 additions & 0 deletions packages/analytics-core/test/plugins/destination.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Response } from '../../src/types/response';
import { API_KEY, useDefaultConfig } from '../helpers/default';
import {
INVALID_API_KEY,
MAX_RETRIES_EXCEEDED_MESSAGE,
MISSING_API_KEY_MESSAGE,
SUCCESS_MESSAGE,
UNEXPECTED_ERROR_MESSAGE,
Expand Down Expand Up @@ -148,6 +149,103 @@ describe('destination', () => {
});
});

describe('offline on max retries (SDKW-42)', () => {
test('getRetryBackoff grows exponentially (bounded by flushMaxRetries, no interval cap)', () => {
const destination = new Destination();
destination.retryTimeout = 1000;
expect(destination.getRetryBackoff(1)).toBe(1000); // 1000 * 2^0
expect(destination.getRetryBackoff(2)).toBe(2000); // 1000 * 2^1
expect(destination.getRetryBackoff(3)).toBe(4000); // 1000 * 2^2
expect(destination.getRetryBackoff(5)).toBe(16000); // 1000 * 2^4 (last retry at default flushMaxRetries=5)
});

test('transient failures below the retry limit use exponential backoff and stay online', () => {
const destination = new Destination();
destination.config = { ...useDefaultConfig(), flushMaxRetries: 20, offline: false };
destination.isClientSide = true; // client SDK (browser / RN)
destination.retryTimeout = 1000;
const schedule = jest.spyOn(destination, 'schedule').mockImplementation(jest.fn);
const context: Context = { event: { event_type: 'a' }, attempts: 0, callback: () => undefined, timeout: 0 };

destination.handleOtherResponse([context]); // attempts -> 1
expect(context.timeout).toBe(1000);
destination.handleOtherResponse([context]); // attempts -> 2
expect(context.timeout).toBe(2000);
destination.handleOtherResponse([context]); // attempts -> 3
expect(context.timeout).toBe(4000);

expect(destination.config.offline).toBe(false);
expect(schedule).toHaveBeenCalled();
});

test('goes offline and retains events instead of dropping once the retry limit is hit', () => {
const diagnosticsClient = new DiagnosticsClient(API_KEY, getMockLogger());
const increment = jest.spyOn(diagnosticsClient, 'increment').mockImplementation(jest.fn);
const destination = new Destination({ diagnosticsClient });
destination.config = { ...useDefaultConfig(), flushMaxRetries: 1, offline: false };
destination.isClientSide = true; // client SDK (browser / RN)
const fulfillRequest = jest.spyOn(destination, 'fulfillRequest').mockImplementation(jest.fn);
jest.spyOn(destination, 'schedule').mockImplementation(jest.fn);
const context: Context = { event: { event_type: 'a' }, attempts: 0, callback: () => undefined, timeout: 0 };
destination.queue = [context];

destination.handleOtherResponse([context]); // attempts 0 -> 1, 1 < 1 is false -> offline

// Event is retained, not dropped.
expect(fulfillRequest).not.toHaveBeenCalled();
expect(destination.queue).toContain(context);
// SDK went offline (mirrors mobile); attempts is left as-is (reset happens on reload
// because attempts is not persisted to storage).
expect(destination.config.offline).toBe(true);
expect(context.attempts).toBe(1);
// Diagnostics counter for going offline due to max retries.
expect(increment).toHaveBeenCalledWith('offline.by.max.retries');
});

test('does not poll the server — offline stays set until reconnect or reload', () => {
jest.useFakeTimers();
try {
const destination = new Destination();
destination.config = { ...useDefaultConfig(), flushMaxRetries: 1, offline: false };
destination.isClientSide = true; // client SDK (browser / RN)
jest.spyOn(destination, 'fulfillRequest').mockImplementation(jest.fn);
jest.spyOn(destination, 'schedule').mockImplementation(jest.fn);
const context: Context = { event: { event_type: 'a' }, attempts: 0, callback: () => undefined, timeout: 0 };
destination.queue = [context];

destination.handleOtherResponse([context]);
expect(destination.config.offline).toBe(true);

// No self-recovery timer: advancing time does not bring the SDK back online.
jest.advanceTimersByTime(10 * 60 * 1000);
expect(destination.config.offline).toBe(true);
} finally {
jest.clearAllTimers();
jest.useRealTimers();
}
});

test('Node (server) SDK drops after max retries instead of going offline', () => {
const destination = new Destination();
destination.config = { ...useDefaultConfig(), flushMaxRetries: 1, offline: false };
destination.isClientSide = false; // server SDK (Node): no reconnect/reload
const fulfillRequest = jest.spyOn(destination, 'fulfillRequest').mockImplementation(jest.fn);
jest.spyOn(destination, 'schedule').mockImplementation(jest.fn);
const context: Context = { event: { event_type: 'a' }, attempts: 0, callback: () => undefined, timeout: 0 };

destination.handleOtherResponse([context]); // attempts 0 -> 1, 1 < 1 is false -> drop

// Legacy behavior: event is dropped, SDK stays online.
expect(fulfillRequest).toHaveBeenCalledWith([context], 500, MAX_RETRIES_EXCEEDED_MESSAGE);
expect(destination.config.offline).toBe(false);
});

test('detects a server (Node/jest) environment as non-recovering on construction', () => {
// jest runs analytics-core in a node testEnvironment -> not a client SDK.
expect(new Destination().isClientSide).toBe(false);
});
});

describe('schedule', () => {
beforeEach(() => {
jest.useFakeTimers();
Expand Down
109 changes: 109 additions & 0 deletions packages/analytics-core/test/utils/environment.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import * as analyticsCoreModule from '../../src/index';
// Not part of the public API — imported from the module under test directly.
import { isBrowser, isClientSide, isWebWorker } from '../../src/utils/environment';
import * as globalScopeModule from '../../src/global-scope';

type ChromeStub = { runtime?: { id?: string | number } };

// Stands in for a worker global: an instance of the constructor it also exposes.
class FakeWorkerGlobalScope {
WorkerGlobalScope = FakeWorkerGlobalScope;
}

const workerGlobalScope = () => new FakeWorkerGlobalScope() as unknown as typeof globalThis;

describe('isChromeExtension', () => {
const originalChrome = (globalThis as typeof globalThis & { chrome?: ChromeStub }).chrome;

Expand Down Expand Up @@ -72,3 +81,103 @@ describe('isReactNative', () => {
expect(analyticsCoreModule.isReactNative()).toBe(true);
});
});

describe('isBrowser', () => {
let getGlobalScopeSpy: jest.SpyInstance;

beforeEach(() => {
getGlobalScopeSpy = jest.spyOn(globalScopeModule, 'getGlobalScope');
});

afterEach(() => {
getGlobalScopeSpy.mockRestore();
});

test('returns false when globalScope is undefined', () => {
getGlobalScopeSpy.mockReturnValue(undefined);
expect(isBrowser()).toBe(false);
});

test('returns false when document is undefined', () => {
getGlobalScopeSpy.mockReturnValue({} as typeof globalThis);
expect(isBrowser()).toBe(false);
});

test('returns true when document is defined', () => {
getGlobalScopeSpy.mockReturnValue({ document: {} } as unknown as typeof globalThis);
expect(isBrowser()).toBe(true);
});
});

describe('isWebWorker', () => {
let getGlobalScopeSpy: jest.SpyInstance;

beforeEach(() => {
getGlobalScopeSpy = jest.spyOn(globalScopeModule, 'getGlobalScope');
});

afterEach(() => {
getGlobalScopeSpy.mockRestore();
});

test('returns false when globalScope is undefined', () => {
getGlobalScopeSpy.mockReturnValue(undefined);
expect(isWebWorker()).toBe(false);
});

test('returns false when WorkerGlobalScope is not exposed', () => {
getGlobalScopeSpy.mockReturnValue({} as typeof globalThis);
expect(isWebWorker()).toBe(false);
});

test('returns false when WorkerGlobalScope is exposed but the scope is not a worker', () => {
// e.g. a server runtime that exposes web constructors on its global.
getGlobalScopeSpy.mockReturnValue({
WorkerGlobalScope: FakeWorkerGlobalScope,
} as unknown as typeof globalThis);
expect(isWebWorker()).toBe(false);
});

test('returns true inside a worker global scope', () => {
getGlobalScopeSpy.mockReturnValue(workerGlobalScope());
expect(isWebWorker()).toBe(true);
});
});

describe('isClientSide', () => {
let getGlobalScopeSpy: jest.SpyInstance;

beforeEach(() => {
getGlobalScopeSpy = jest.spyOn(globalScopeModule, 'getGlobalScope');
});

afterEach(() => {
getGlobalScopeSpy.mockRestore();
});

test('returns true in a React Native environment', () => {
getGlobalScopeSpy.mockReturnValue({ navigator: { product: 'ReactNative' } } as unknown as typeof globalThis);
expect(isClientSide()).toBe(true);
});

test('returns true in a browser environment', () => {
getGlobalScopeSpy.mockReturnValue({ document: {} } as unknown as typeof globalThis);
expect(isClientSide()).toBe(true);
});

test('returns true in a document-less browser worker', () => {
// e.g. a Manifest V3 extension background service worker.
getGlobalScopeSpy.mockReturnValue(workerGlobalScope());
expect(isClientSide()).toBe(true);
});

test('returns true in a Chrome extension without document', () => {
getGlobalScopeSpy.mockReturnValue({ chrome: { runtime: { id: 'ext-abc' } } } as unknown as typeof globalThis);
expect(isClientSide()).toBe(true);
});

test('returns false in a server (Node) environment', () => {
getGlobalScopeSpy.mockReturnValue({} as typeof globalThis);
expect(isClientSide()).toBe(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ describe('migration', () => {
migrateLegacyData: false,
loggerProvider,
instanceName: 'test-instance',
attribution: {
disabled: true,
},
}).promise;
expect(client.getDeviceId()).not.toEqual(deviceId);
expect(client.getUserId()).not.toEqual(userId);
Expand Down
Loading
Loading