From 500f01d8aa4fe5f112a556758a28bb4efc766583 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 9 Apr 2026 18:40:01 +0000 Subject: [PATCH 1/2] fix(analytics-browser): auto-flush localhost uploads Co-authored-by: amp-opportunities --- packages/analytics-browser/CHANGELOG.md | 1 + packages/analytics-browser/src/config.ts | 15 +++++- .../analytics-browser/test/config.test.ts | 50 +++++++++++++++++++ .../analytics-core/src/plugins/destination.ts | 33 ++++++++++-- .../src/types/config/core-config.ts | 5 ++ .../test/plugins/destination.test.ts | 41 +++++++++++++++ packages/analytics-types/src/config/core.ts | 5 ++ 7 files changed, 143 insertions(+), 7 deletions(-) diff --git a/packages/analytics-browser/CHANGELOG.md b/packages/analytics-browser/CHANGELOG.md index 2b0d45dc38..cd354a4173 100644 --- a/packages/analytics-browser/CHANGELOG.md +++ b/packages/analytics-browser/CHANGELOG.md @@ -8,6 +8,7 @@ See [Conventional Commits](https://conventionalcommits.org) for commit guideline ### Bug Fixes +* **analytics-browser:** auto-flush immediately on localhost and surface upload errors via `onUploadError` * **plugin-custom-enrichment:** allow disable custom enrichment plugin over remote config settings ([#1638](https://github.com/amplitude/Amplitude-TypeScript/issues/1638)) ([385c4de](https://github.com/amplitude/Amplitude-TypeScript/commit/385c4ded2b2622fde1ac0930495805e11353d55f)) diff --git a/packages/analytics-browser/src/config.ts b/packages/analytics-browser/src/config.ts index f4b90c89bd..139cd672f5 100644 --- a/packages/analytics-browser/src/config.ts +++ b/packages/analytics-browser/src/config.ts @@ -47,6 +47,7 @@ import { getDomain, KNOWN_2LDS } from './attribution/helpers'; // Exported for testing purposes only. Do not expose to public interface. export class BrowserConfig extends Config implements IBrowserConfig { public readonly version = VERSION; + declare onUploadError?: IBrowserConfig['onUploadError']; protected _cookieStorage: Storage; protected _deviceId?: string; protected _lastEventId?: number; @@ -292,6 +293,11 @@ export interface EarlyConfig { diagnosticsSampleRate: number; } +const isLocalhost = (hostname?: string) => { + const currentHostname = hostname ?? (typeof location === 'undefined' ? undefined : location.hostname); + return currentHostname === 'localhost' || currentHostname === '127.0.0.1'; +}; + export const useBrowserConfig = async ( apiKey: string, options: BrowserOptions = {}, @@ -366,6 +372,7 @@ export const useBrowserConfig = async ( language: options.trackingOptions?.language ?? true, platform: options.trackingOptions?.platform ?? true, }; + const shouldUseLocalhostFlushDefaults = isLocalhost(); const pageCounter = previousCookies?.pageCounter; const debugLogsEnabled = previousCookies?.debugLogsEnabled; @@ -382,9 +389,9 @@ export const useBrowserConfig = async ( options.defaultTracking, options.autocapture, deviceId, - options.flushIntervalMillis, + options.flushIntervalMillis ?? (shouldUseLocalhostFlushDefaults ? 0 : undefined), options.flushMaxRetries, - options.flushQueueSize, + options.flushQueueSize ?? (shouldUseLocalhostFlushDefaults ? 1 : undefined), identityStorage, options.ingestionMetadata, options.instanceName, @@ -425,6 +432,10 @@ export const useBrowserConfig = async ( options.customEnrichment, ); + if (options.onUploadError) { + browserConfig.onUploadError = options.onUploadError; + } + if (!(await browserConfig.storageProvider.isEnabled())) { browserConfig.loggerProvider.warn( `Storage provider ${browserConfig.storageProvider.constructor.name} is not enabled. Falling back to MemoryStorage.`, diff --git a/packages/analytics-browser/test/config.test.ts b/packages/analytics-browser/test/config.test.ts index 3ca5e6068c..e892b95126 100644 --- a/packages/analytics-browser/test/config.test.ts +++ b/packages/analytics-browser/test/config.test.ts @@ -101,6 +101,22 @@ describe('config', () => { }); describe('useBrowserConfig', () => { + const originalLocation = window.location; + + beforeEach(() => { + Object.defineProperty(window, 'location', { + value: { hostname: 'amplitude.com' }, + configurable: true, + }); + }); + + afterEach(() => { + Object.defineProperty(window, 'location', { + value: originalLocation, + configurable: true, + }); + }); + test('should create default config', async () => { const getTopLevelDomain = jest.spyOn(Config, 'getTopLevelDomain').mockResolvedValueOnce('.amplitude.com'); const logger = new core.Logger(); @@ -162,6 +178,40 @@ describe('config', () => { expect(getTopLevelDomain).toHaveBeenCalledTimes(1); }); + test.each(['localhost', '127.0.0.1'])( + 'should enable immediate flush defaults on %s', + async (hostname: string) => { + Object.defineProperty(window, 'location', { + value: { hostname }, + configurable: true, + }); + + const config = await Config.useBrowserConfig(apiKey, undefined, new AmplitudeBrowser()); + + expect(config.flushIntervalMillis).toBe(0); + expect(config.flushQueueSize).toBe(1); + }, + ); + + test('should allow localhost flush defaults to be overridden explicitly', async () => { + Object.defineProperty(window, 'location', { + value: { hostname: 'localhost' }, + configurable: true, + }); + + const config = await Config.useBrowserConfig( + apiKey, + { + flushIntervalMillis: 5000, + flushQueueSize: 10, + }, + new AmplitudeBrowser(), + ); + + expect(config.flushIntervalMillis).toBe(5000); + expect(config.flushQueueSize).toBe(10); + }); + test('should fall back to memoryStorage when storageProvider is not enabled', async () => { const localStorageIsEnabledSpy = jest .spyOn(LocalStorageModule.LocalStorage.prototype, 'isEnabled') diff --git a/packages/analytics-core/src/plugins/destination.ts b/packages/analytics-core/src/plugins/destination.ts index 1a6758359f..ef07c41266 100644 --- a/packages/analytics-core/src/plugins/destination.ts +++ b/packages/analytics-core/src/plugins/destination.ts @@ -60,6 +60,14 @@ function getErrorMessage(error: unknown) { return String(error); } +function getResponseMessage(res: Response) { + if ('body' in res) { + return `${res.status}: ${getResponseBodyString(res)}`; + } + + return res.status; +} + export function getResponseBodyString(res: Response) { let responseBodyString = ''; try { @@ -137,6 +145,7 @@ export class Destination implements DestinationPlugin { if (context.attempts < this.config.flushMaxRetries) { return true; } + this.notifyUploadError(MAX_RETRIES_EXCEEDED_MESSAGE); void this.fulfillRequest([context], 500, MAX_RETRIES_EXCEEDED_MESSAGE); return false; }); @@ -248,21 +257,23 @@ export class Destination implements DestinationPlugin { this.config._enableRequestBodyCompressionExperimental && shouldCompressUploadBody, ); if (res === null) { + this.notifyUploadError(UNEXPECTED_ERROR_MESSAGE); this.fulfillRequest(list, 0, UNEXPECTED_ERROR_MESSAGE); return; } + const responseMessage = getResponseMessage(res); + if (res.statusCode === 0) { + this.notifyUploadError(responseMessage); + } if (!useRetry) { - if ('body' in res) { - this.fulfillRequest(list, res.statusCode, `${res.status}: ${getResponseBodyString(res)}`); - } else { - this.fulfillRequest(list, res.statusCode, res.status); - } + this.fulfillRequest(list, res.statusCode, responseMessage); return; } this.handleResponse(res, list); } catch (e) { const errorMessage = getErrorMessage(e); this.config.loggerProvider.error(errorMessage); + this.notifyUploadError(errorMessage); this.diagnosticsClient?.recordEvent('analytics.events.unsuccessful.from.catch.error', { events: list.map((context) => context.event.event_type), @@ -421,6 +432,18 @@ export class Destination implements DestinationPlugin { list.forEach((context) => context.callback(buildResult(context.event, code, message))); } + notifyUploadError(message: string) { + if (!this.config.onUploadError) { + return; + } + + try { + this.config.onUploadError(message); + } catch (error) { + this.config.loggerProvider.error('Error calling onUploadError callback', error); + } + } + /** * This is called on * 1) new events are added to queue; or diff --git a/packages/analytics-core/src/types/config/core-config.ts b/packages/analytics-core/src/types/config/core-config.ts index 43ffbd4d19..f27545d4df 100644 --- a/packages/analytics-core/src/types/config/core-config.ts +++ b/packages/analytics-core/src/types/config/core-config.ts @@ -39,6 +39,11 @@ export interface IConfig { * A custom Logger class to emit log messages to desired destination. */ loggerProvider: ILogger; + /** + * Called when an upload fails because the browser could not reach the endpoint + * or when the SDK drops the event after exhausting retries. + */ + onUploadError?: (error: string) => void; /** * The minimum length for the value of userId and deviceId properties. */ diff --git a/packages/analytics-core/test/plugins/destination.test.ts b/packages/analytics-core/test/plugins/destination.test.ts index aa94c181c7..9760df1fcb 100644 --- a/packages/analytics-core/test/plugins/destination.test.ts +++ b/packages/analytics-core/test/plugins/destination.test.ts @@ -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, @@ -146,6 +147,27 @@ describe('destination', () => { expect(result.length).toBe(1); expect(result[0].event.event_type).toBe('event_3'); }); + + test('should call onUploadError when retries are exhausted', () => { + const onUploadError = jest.fn(); + const destination = new Destination(); + destination.config = { + ...useDefaultConfig(), + flushMaxRetries: 1, + onUploadError, + }; + + destination.removeEventsExceedFlushMaxRetries([ + { + event: { event_type: 'event_1' }, + attempts: 0, + callback: () => undefined, + timeout: 0, + }, + ]); + + expect(onUploadError).toHaveBeenCalledWith(MAX_RETRIES_EXCEEDED_MESSAGE); + }); }); describe('schedule', () => { @@ -912,6 +934,25 @@ describe('destination', () => { expect(Array.isArray(recordEventCall[1].stack_trace)).toBe(true); expect(recordEventCall[1].stack_trace.length).toBeGreaterThan(0); }); + + test('should call onUploadError when send throws an error', async () => { + const onUploadError = jest.fn(); + const destination = new Destination(); + const transportProvider = { + send: jest.fn().mockImplementationOnce(() => { + throw new Error('Network error'); + }), + }; + await destination.setup({ + ...useDefaultConfig(), + onUploadError, + transportProvider, + }); + + await destination.send([{ event: { event_type: 'event1' }, attempts: 0, callback: jest.fn(), timeout: 0 }]); + + expect(onUploadError).toHaveBeenCalledWith('Network error'); + }); }); describe('handleResponse', () => { diff --git a/packages/analytics-types/src/config/core.ts b/packages/analytics-types/src/config/core.ts index 55ffc9fc76..5ba4b89901 100644 --- a/packages/analytics-types/src/config/core.ts +++ b/packages/analytics-types/src/config/core.ts @@ -37,6 +37,11 @@ export interface Config { * A custom Logger class to emit log messages to desired destination. */ loggerProvider: Logger; + /** + * Called when an upload fails because the browser could not reach the endpoint + * or when the SDK drops the event after exhausting retries. + */ + onUploadError?: (error: string) => void; /** * The minimum length for the value of userId and deviceId properties. */ From 4ce30fc696dbf0404c2d74da55e59a0316665475 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 9 Apr 2026 18:47:28 +0000 Subject: [PATCH 2/2] test(analytics-browser): tidy config test imports --- .../analytics-browser/test/config.test.ts | 87 +++++++++---------- 1 file changed, 41 insertions(+), 46 deletions(-) diff --git a/packages/analytics-browser/test/config.test.ts b/packages/analytics-browser/test/config.test.ts index e892b95126..ecbe23f4d5 100644 --- a/packages/analytics-browser/test/config.test.ts +++ b/packages/analytics-browser/test/config.test.ts @@ -1,18 +1,18 @@ import * as Config from '../src/config'; import * as LocalStorageModule from '../src/storage/local-storage'; import * as SessionStorageModule from '../src/storage/session-storage'; -import * as core from '@amplitude/analytics-core'; import { + BrowserConfig, + CookieStorage, + FetchTransport as CoreFetchTransport, + getCookieName, LogLevel, + Logger, + MemoryStorage, Storage, UserSession, - MemoryStorage, - getCookieName, - FetchTransport as CoreFetchTransport, - Logger, - BrowserConfig, + UUID, } from '@amplitude/analytics-core'; -import * as BrowserUtils from '@amplitude/analytics-core'; import { XHRTransport } from '../src/transports/xhr'; import { createTransport, useBrowserConfig, shouldFetchRemoteConfig } from '../src/config'; import { FetchTransport } from '../src/transports/fetch'; @@ -24,9 +24,7 @@ import { VERSION } from '../src/version'; describe('config', () => { const someUUID: string = expect.stringMatching(uuidPattern) as string; - const someCookieStorage: BrowserUtils.CookieStorage = expect.any( - BrowserUtils.CookieStorage, - ) as BrowserUtils.CookieStorage; + const someCookieStorage: CookieStorage = expect.any(CookieStorage) as CookieStorage; const someLocalStorage: LocalStorageModule.LocalStorage = expect.any( LocalStorageModule.LocalStorage, ) as LocalStorageModule.LocalStorage; @@ -34,7 +32,7 @@ describe('config', () => { let apiKey = ''; beforeEach(() => { - apiKey = core.UUID(); + apiKey = UUID(); }); describe('BrowserConfig', () => { @@ -119,7 +117,7 @@ describe('config', () => { test('should create default config', async () => { const getTopLevelDomain = jest.spyOn(Config, 'getTopLevelDomain').mockResolvedValueOnce('.amplitude.com'); - const logger = new core.Logger(); + const logger = new Logger(); logger.enable(LogLevel.Warn); const config = await Config.useBrowserConfig(apiKey, undefined, new AmplitudeBrowser()); expect(config).toEqual({ @@ -178,20 +176,17 @@ describe('config', () => { expect(getTopLevelDomain).toHaveBeenCalledTimes(1); }); - test.each(['localhost', '127.0.0.1'])( - 'should enable immediate flush defaults on %s', - async (hostname: string) => { - Object.defineProperty(window, 'location', { - value: { hostname }, - configurable: true, - }); + test.each(['localhost', '127.0.0.1'])('should enable immediate flush defaults on %s', async (hostname: string) => { + Object.defineProperty(window, 'location', { + value: { hostname }, + configurable: true, + }); - const config = await Config.useBrowserConfig(apiKey, undefined, new AmplitudeBrowser()); + const config = await Config.useBrowserConfig(apiKey, undefined, new AmplitudeBrowser()); - expect(config.flushIntervalMillis).toBe(0); - expect(config.flushQueueSize).toBe(1); - }, - ); + expect(config.flushIntervalMillis).toBe(0); + expect(config.flushQueueSize).toBe(1); + }); test('should allow localhost flush defaults to be overridden explicitly', async () => { Object.defineProperty(window, 'location', { @@ -216,7 +211,7 @@ describe('config', () => { const localStorageIsEnabledSpy = jest .spyOn(LocalStorageModule.LocalStorage.prototype, 'isEnabled') .mockResolvedValueOnce(false); - const loggerProviderSpy = jest.spyOn(core.Logger.prototype, 'warn'); + const loggerProviderSpy = jest.spyOn(Logger.prototype, 'warn'); const config = await Config.useBrowserConfig(apiKey, undefined, new AmplitudeBrowser()); expect(localStorageIsEnabledSpy).toHaveBeenCalledTimes(1); expect(loggerProviderSpy).toHaveBeenCalledWith( @@ -243,7 +238,7 @@ describe('config', () => { }, configurable: true, }); - const cookieStorage = new core.MemoryStorage(); + const cookieStorage = new MemoryStorage(); await cookieStorage.set(getCookieName(apiKey), { deviceId: 'device-device-device', sessionId: -1, @@ -252,7 +247,7 @@ describe('config', () => { lastEventTime: 1, optOut: false, }); - const logger = new core.Logger(); + const logger = new Logger(); logger.enable(LogLevel.Warn); jest.spyOn(Config, 'createCookieStorage').mockReturnValueOnce(cookieStorage); const config = await Config.useBrowserConfig( @@ -426,7 +421,7 @@ describe('config', () => { describe('createCookieStorage', () => { test('should return cookies', async () => { const storage = Config.createCookieStorage(DEFAULT_IDENTITY_STORAGE); - expect(storage).toBeInstanceOf(BrowserUtils.CookieStorage); + expect(storage).toBeInstanceOf(CookieStorage); }); test('should use return storage', async () => { @@ -441,7 +436,7 @@ describe('config', () => { test('should use memory', async () => { const storage = Config.createCookieStorage('none'); - expect(storage).toBeInstanceOf(core.MemoryStorage); + expect(storage).toBeInstanceOf(MemoryStorage); }); }); @@ -487,7 +482,7 @@ describe('config', () => { describe('getTopLevelDomain', () => { test('should return empty string for localhost', async () => { - const isDomainWritableSpy = jest.spyOn(BrowserUtils.CookieStorage, 'isDomainWritable'); + const isDomainWritableSpy = jest.spyOn(CookieStorage, 'isDomainWritable'); const domain = await Config.getTopLevelDomain(undefined); expect(isDomainWritableSpy).not.toHaveBeenCalled(); expect(domain).toBe(''); @@ -495,7 +490,7 @@ describe('config', () => { }); test('should return empty string for single part hostname', async () => { - const isDomainWritableSpy = jest.spyOn(BrowserUtils.CookieStorage, 'isDomainWritable'); + const isDomainWritableSpy = jest.spyOn(CookieStorage, 'isDomainWritable'); const domain = await Config.getTopLevelDomain('mylocaldomain'); expect(isDomainWritableSpy).not.toHaveBeenCalled(); expect(domain).toBe(''); @@ -510,11 +505,11 @@ describe('config', () => { remove: jest.fn().mockResolvedValueOnce(Promise.resolve(undefined)), reset: jest.fn().mockResolvedValueOnce(Promise.resolve(undefined)), }; - jest.spyOn(BrowserUtils, 'CookieStorage').mockReturnValueOnce({ + jest.spyOn({ CookieStorage }, 'CookieStorage').mockReturnValueOnce({ ...testCookieStorage, options: {}, config: {}, - } as unknown as BrowserUtils.CookieStorage); + } as unknown as CookieStorage); const domain = await Config.getTopLevelDomain(); expect(domain).toBe(''); }); @@ -537,24 +532,24 @@ describe('config', () => { reset: jest.fn().mockResolvedValue(Promise.resolve(undefined)), // CookieStorage.transaction is used by getTopLevelDomain; first domain fails, second succeeds transaction: jest.fn().mockResolvedValueOnce(false).mockResolvedValueOnce(true), - } as unknown as BrowserUtils.CookieStorage; + } as unknown as CookieStorage; jest - .spyOn(BrowserUtils, 'CookieStorage') + .spyOn({ CookieStorage }, 'CookieStorage') .mockReturnValueOnce({ ...testCookieStorage, options: {}, config: {}, - } as unknown as BrowserUtils.CookieStorage) + } as unknown as CookieStorage) .mockReturnValue({ ...actualCookieStorage, options: {}, config: {}, - } as unknown as BrowserUtils.CookieStorage); + } as unknown as CookieStorage); // eslint-disable-next-line @typescript-eslint/unbound-method - const isDomainWritableBefore = BrowserUtils.CookieStorage.isDomainWritable; + const isDomainWritableBefore = CookieStorage.isDomainWritable; try { - BrowserUtils.CookieStorage.isDomainWritable = jest.fn().mockImplementation((domain: string) => { + CookieStorage.isDomainWritable = jest.fn().mockImplementation((domain: string) => { if (domain === 'gov.uk') return Promise.resolve(false); if (domain === 'ac.be') return Promise.resolve(false); if (domain === 'legislation.gov.uk') return Promise.resolve(true); @@ -566,7 +561,7 @@ describe('config', () => { expect(await Config.getTopLevelDomain('www.website.com')).toBe('.website.com'); expect(await Config.getTopLevelDomain('www.hello.ac.be')).toBe('.hello.ac.be'); } finally { - BrowserUtils.CookieStorage.isDomainWritable = isDomainWritableBefore; + CookieStorage.isDomainWritable = isDomainWritableBefore; } }); @@ -629,16 +624,16 @@ describe('config', () => { }; // eslint-disable-next-line @typescript-eslint/unbound-method - const isDomainWritableBefore = BrowserUtils.CookieStorage.isDomainWritable; + const isDomainWritableBefore = CookieStorage.isDomainWritable; try { - BrowserUtils.CookieStorage.isDomainWritable = jest.fn().mockRejectedValue(tldError); + CookieStorage.isDomainWritable = jest.fn().mockRejectedValue(tldError); expect(await Config.getTopLevelDomain('www.example.com', mockDiagnosticsClient)).toBe(''); expect(mockDiagnosticsClient.recordEvent).toHaveBeenCalledWith('cookies.tld.failure', { reason: 'Unexpected exception checking domain is writable: example.com', error: 'cookie access denied', }); } finally { - BrowserUtils.CookieStorage.isDomainWritable = isDomainWritableBefore; + CookieStorage.isDomainWritable = isDomainWritableBefore; } }); }); @@ -678,7 +673,7 @@ describe('config', () => { const encodeJson = (session: UserSession) => btoa(encodeURIComponent(JSON.stringify(session))); let config: BrowserConfig; - let cookieStorage: BrowserUtils.CookieStorage; + let cookieStorage: CookieStorage; let duplicateResolverFn: ((value: string) => boolean) | undefined; beforeEach(async () => { @@ -687,7 +682,7 @@ describe('config', () => { { cookieOptions: { domain: '.amplitude.com' } }, new AmplitudeBrowser(), ); - cookieStorage = config.cookieStorage as BrowserUtils.CookieStorage; + cookieStorage = config.cookieStorage as CookieStorage; duplicateResolverFn = cookieStorage.config.duplicateResolverFn; }); @@ -732,7 +727,7 @@ describe('config', () => { describe('useBrowserConfig with earlyConfig', () => { test('should use earlyConfig values when provided', async () => { - const customLogger = new core.Logger(); + const customLogger = new Logger(); customLogger.enable(LogLevel.Debug); const earlyConfig: Config.EarlyConfig = {