diff --git a/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts b/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts index 3a50c40dd8..d9c1af07f1 100644 --- a/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts +++ b/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts @@ -657,3 +657,79 @@ it('only logs error filter error once', () => { expect(mockLogger.warn).toHaveBeenCalledTimes(1); }); + +it('waits for client initialization before sending events', async () => { + const telemetry = new BrowserTelemetryImpl(defaultOptions); + const error = new Error('Test error'); + + let resolver; + + const initPromise = new Promise((resolve) => { + resolver = resolve; + }); + + const mockInitClient = { + track: jest.fn(), + waitForInitialization: jest.fn().mockImplementation(() => initPromise), + }; + + telemetry.captureError(error); + telemetry.register(mockInitClient); + + expect(mockInitClient.track).not.toHaveBeenCalled(); + + resolver!(); + + await initPromise; + + expect(mockInitClient.track).toHaveBeenCalledWith( + '$ld:telemetry:session:init', + expect.objectContaining({ + sessionId: expect.any(String), + }), + ); + + expect(mockInitClient.track).toHaveBeenCalledWith( + '$ld:telemetry:error', + expect.objectContaining({ + type: 'Error', + message: 'Test error', + stack: { frames: expect.any(Array) }, + breadcrumbs: [], + sessionId: expect.any(String), + }), + ); +}); + +it('handles client initialization failure gracefully', async () => { + const telemetry = new BrowserTelemetryImpl(defaultOptions); + const error = new Error('Test error'); + const mockInitClient = { + track: jest.fn(), + waitForInitialization: jest.fn().mockRejectedValue(new Error('Init failed')), + }; + + telemetry.captureError(error); + telemetry.register(mockInitClient); + + await expect(mockInitClient.waitForInitialization()).rejects.toThrow('Init failed'); + + // Should still send events even if initialization fails + expect(mockInitClient.track).toHaveBeenCalledWith( + '$ld:telemetry:session:init', + expect.objectContaining({ + sessionId: expect.any(String), + }), + ); + + expect(mockInitClient.track).toHaveBeenCalledWith( + '$ld:telemetry:error', + expect.objectContaining({ + type: 'Error', + message: 'Test error', + stack: { frames: expect.any(Array) }, + breadcrumbs: [], + sessionId: expect.any(String), + }), + ); +}); diff --git a/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts b/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts index 1b08d881bb..3e39254a33 100644 --- a/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts +++ b/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts @@ -5,7 +5,7 @@ */ import type { LDContext, LDEvaluationDetail } from '@launchdarkly/js-client-sdk'; -import { LDClientLogging, LDClientTracking, MinLogger } from './api'; +import { LDClientInitialization, LDClientLogging, LDClientTracking, MinLogger } from './api'; import { Breadcrumb, FeatureManagementBreadcrumb } from './api/Breadcrumb'; import { BrowserTelemetry } from './api/BrowserTelemetry'; import { BrowserTelemetryInspector } from './api/client/BrowserTelemetryInspector'; @@ -34,6 +34,12 @@ const GENERIC_EXCEPTION = 'generic'; const NULL_EXCEPTION_MESSAGE = 'exception was null or undefined'; const MISSING_MESSAGE = 'exception had no message'; +// Timeout for client initialization. The telemetry SDK doesn't require that the client be initialized, but it does +// require that the context processing that happens during initialization complete. This is some subset of the total +// initialization time, but we don't care if initialization actually completes within the, just that the context +// is available for event sending. +const INITIALIZATION_TIMEOUT = 5; + /** * Given a flag value ensure it is safe for analytics. * @@ -83,6 +89,10 @@ function isLDClientLogging(client: unknown): client is LDClientLogging { return (client as any).logger !== undefined; } +function isLDClientInitialization(client: unknown): client is LDClientInitialization { + return (client as any).waitForInitialization !== undefined; +} + export default class BrowserTelemetryImpl implements BrowserTelemetry { private _maxPendingEvents: number; private _maxBreadcrumbs: number; @@ -98,6 +108,10 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { private _logger: MinLogger; + private _registrationComplete: boolean = false; + + // Used to ensure we only log the event dropped message once. + private _clientRegistered: boolean = false; // Used to ensure we only log the event dropped message once. private _eventsDropped: boolean = false; // Used to ensure we only log the breadcrumb filter error once. @@ -159,17 +173,45 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { } register(client: LDClientTracking): void { + if (this._client !== undefined) { + return; + } + this._client = client; + // When the client is registered, we need to set the logger again, because we may be able to use the client's // logger. this._setLogger(); - this._client.track(SESSION_INIT_KEY, { sessionId: this._sessionId }); + const completeRegistration = () => { + this._client?.track(SESSION_INIT_KEY, { sessionId: this._sessionId }); - this._pendingEvents.forEach((event) => { - this._client?.track(event.type, event.data); - }); - this._pendingEvents = []; + this._pendingEvents.forEach((event) => { + this._client?.track(event.type, event.data); + }); + this._pendingEvents = []; + this._registrationComplete = true; + }; + + if (isLDClientInitialization(client)) { + // We don't actually need the client initialization to complete, but we do need the context processing that + // happens during initialization to complete. This time will be some time greater than that, but we don't + // care if initialization actually completes within the timeout. + + // An immediately invoked async function is used to ensure that the registration method can be called synchronously. + // Making the `register` method async would increase the complexity for application developers. + (async () => { + try { + await client.waitForInitialization(INITIALIZATION_TIMEOUT); + } catch { + // We don't care if the initialization fails. + } + completeRegistration(); + })(); + } else { + // TODO(EMSR-36): Figure out how to handle the 4.x implementation. + completeRegistration(); + } } private _setLogger() { @@ -207,7 +249,9 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { return; } - if (this._client === undefined) { + if (this._registrationComplete) { + this._client?.track(type, filteredEvent); + } else { this._pendingEvents.push({ type, data: filteredEvent }); if (this._pendingEvents.length > this._maxPendingEvents) { if (!this._eventsDropped) { @@ -221,7 +265,6 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { this._pendingEvents.shift(); } } - this._client?.track(type, filteredEvent); } captureError(exception: Error): void { diff --git a/packages/telemetry/browser-telemetry/src/api/client/LDClientInitialization.ts b/packages/telemetry/browser-telemetry/src/api/client/LDClientInitialization.ts new file mode 100644 index 0000000000..acf6f578cb --- /dev/null +++ b/packages/telemetry/browser-telemetry/src/api/client/LDClientInitialization.ts @@ -0,0 +1,6 @@ +/** + * Minimal client interface which allows waiting for initialization. + */ +export interface LDClientInitialization { + waitForInitialization(timeout?: number): Promise; +} diff --git a/packages/telemetry/browser-telemetry/src/api/client/index.ts b/packages/telemetry/browser-telemetry/src/api/client/index.ts index e3a053ff40..8ed56f88c8 100644 --- a/packages/telemetry/browser-telemetry/src/api/client/index.ts +++ b/packages/telemetry/browser-telemetry/src/api/client/index.ts @@ -1,3 +1,4 @@ export * from './LDClientTracking'; export * from './LDClientLogging'; export * from './BrowserTelemetryInspector'; +export * from './LDClientInitialization';