diff --git a/.changeset/thirty-cars-beg.md b/.changeset/thirty-cars-beg.md new file mode 100644 index 00000000000..755e516bf69 --- /dev/null +++ b/.changeset/thirty-cars-beg.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': patch +--- + +Do not allow errors in telemetry to bubble up diff --git a/packages/shared/src/__tests__/telemetry.logs.test.ts b/packages/shared/src/__tests__/telemetry.logs.test.ts index 695ddbaa60c..a5602de1567 100644 --- a/packages/shared/src/__tests__/telemetry.logs.test.ts +++ b/packages/shared/src/__tests__/telemetry.logs.test.ts @@ -49,7 +49,6 @@ describe('TelemetryCollector.recordLog', () => { expect(log.iid).toBeUndefined(); expect(log.ts).toBe(new Date(ts).toISOString()); expect(log.pk).toBe(TEST_PK); - // Function and undefined stripped out expect(log.payload).toEqual({ a: 1 }); }); @@ -62,7 +61,6 @@ describe('TelemetryCollector.recordLog', () => { timestamp: Date.now(), }; - // undefined context fetchSpy.mockClear(); collector.recordLog({ ...base, context: undefined } as any); jest.runAllTimers(); @@ -71,7 +69,6 @@ describe('TelemetryCollector.recordLog', () => { let body = JSON.parse(initOptions1.body as string); expect(body.logs[0].payload).toBeNull(); - // array context fetchSpy.mockClear(); collector.recordLog({ ...base, context: [1, 2, 3] } as any); jest.runAllTimers(); @@ -80,7 +77,6 @@ describe('TelemetryCollector.recordLog', () => { body = JSON.parse(initOptions2.body as string); expect(body.logs[0].payload).toBeNull(); - // circular context fetchSpy.mockClear(); const circular: any = { foo: 'bar' }; circular.self = circular; @@ -95,7 +91,6 @@ describe('TelemetryCollector.recordLog', () => { test('drops invalid entries: missing id, invalid level, empty message, invalid timestamp', () => { const collector = new TelemetryCollector({ publishableKey: TEST_PK }); - // invalid level fetchSpy.mockClear(); collector.recordLog({ level: 'fatal' as unknown as any, @@ -105,7 +100,6 @@ describe('TelemetryCollector.recordLog', () => { jest.runAllTimers(); expect(fetchSpy).not.toHaveBeenCalled(); - // empty message fetchSpy.mockClear(); collector.recordLog({ level: 'debug', @@ -115,7 +109,6 @@ describe('TelemetryCollector.recordLog', () => { jest.runAllTimers(); expect(fetchSpy).not.toHaveBeenCalled(); - // invalid timestamp (NaN) fetchSpy.mockClear(); collector.recordLog({ level: 'warn', @@ -144,4 +137,86 @@ describe('TelemetryCollector.recordLog', () => { const body = JSON.parse(initOptions4.body as string); expect(body.logs[0].ts).toBe(new Date(tsString).toISOString()); }); + + describe('error handling', () => { + test('recordLog() method handles circular references in context gracefully', () => { + const collector = new TelemetryCollector({ publishableKey: TEST_PK }); + + const circularContext = (() => { + const obj: any = { test: 'value' }; + obj.self = obj; + return obj; + })(); + + expect(() => { + collector.recordLog({ + level: 'info', + message: 'test message', + timestamp: Date.now(), + context: circularContext, + }); + }).not.toThrow(); + + jest.runAllTimers(); + expect(fetchSpy).toHaveBeenCalled(); + + const [url, init] = fetchSpy.mock.calls[0]; + expect(String(url)).toMatch('/v1/logs'); + + const initOptions = init as RequestInit; + const body = JSON.parse(initOptions.body as string); + expect(body.logs[0].payload).toBeNull(); + }); + + test('recordLog() method handles non-serializable context gracefully', () => { + const collector = new TelemetryCollector({ publishableKey: TEST_PK }); + + const nonSerializableContext = { + function: () => 'test', + undefined: undefined, + symbol: Symbol('test'), + circular: (() => { + const obj: any = { test: 'value' }; + obj.self = obj; + return obj; + })(), + }; + + expect(() => { + collector.recordLog({ + level: 'info', + message: 'test message', + timestamp: Date.now(), + context: nonSerializableContext, + }); + }).not.toThrow(); + + jest.runAllTimers(); + expect(fetchSpy).toHaveBeenCalled(); + + const [url, init] = fetchSpy.mock.calls[0]; + expect(String(url)).toMatch('/v1/logs'); + + const initOptions = init as RequestInit; + const body = JSON.parse(initOptions.body as string); + expect(body.logs[0].payload).toBeNull(); + }); + + test('recordLog() method handles invalid timestamp gracefully', () => { + const collector = new TelemetryCollector({ publishableKey: TEST_PK }); + + const invalidTimestamp = new Date('invalid date'); + + expect(() => { + collector.recordLog({ + level: 'info', + message: 'test message', + timestamp: invalidTimestamp.getTime(), + }); + }).not.toThrow(); + + jest.runAllTimers(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/shared/src/__tests__/telemetry.test.ts b/packages/shared/src/__tests__/telemetry.test.ts index bbd8b5b4be1..41bc3a77d8c 100644 --- a/packages/shared/src/__tests__/telemetry.test.ts +++ b/packages/shared/src/__tests__/telemetry.test.ts @@ -392,4 +392,63 @@ describe('TelemetryCollector', () => { fetchSpy.mockRestore(); }); }); + + describe('error handling', () => { + test('record() method does not bubble up errors from internal operations', () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const circularPayload = (() => { + const obj: any = { test: 'value' }; + obj.self = obj; + return obj; + })(); + + expect(() => { + collector.record({ event: 'TEST_EVENT', payload: circularPayload }); + }).not.toThrow(); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[clerk/telemetry] Error recording telemetry event', + expect.any(Error), + ); + + consoleErrorSpy.mockRestore(); + }); + + test('record() method handles errors gracefully and continues operation', () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const problematicPayload = (() => { + const obj: any = { test: 'value' }; + obj.self = obj; + return obj; + })(); + + expect(() => { + collector.record({ event: 'TEST_EVENT', payload: problematicPayload }); + }).not.toThrow(); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[clerk/telemetry] Error recording telemetry event', + expect.any(Error), + ); + + expect(() => { + collector.record({ event: 'TEST_EVENT', payload: { normal: 'data' } }); + }).not.toThrow(); + + jest.runAllTimers(); + expect(fetchSpy).toHaveBeenCalled(); + + consoleErrorSpy.mockRestore(); + }); + }); }); diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index cc2f61656be..dad20550105 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -176,17 +176,21 @@ export class TelemetryCollector implements TelemetryCollectorInterface { } record(event: TelemetryEventRaw): void { - const preparedPayload = this.#preparePayload(event.event, event.payload); + try { + const preparedPayload = this.#preparePayload(event.event, event.payload); - this.#logEvent(preparedPayload.event, preparedPayload); + this.#logEvent(preparedPayload.event, preparedPayload); - if (!this.#shouldRecord(preparedPayload, event.eventSamplingRate)) { - return; - } + if (!this.#shouldRecord(preparedPayload, event.eventSamplingRate)) { + return; + } - this.#buffer.push({ kind: 'event', value: preparedPayload }); + this.#buffer.push({ kind: 'event', value: preparedPayload }); - this.#scheduleFlush(); + this.#scheduleFlush(); + } catch (error) { + console.error('[clerk/telemetry] Error recording telemetry event', error); + } } /** @@ -195,49 +199,53 @@ export class TelemetryCollector implements TelemetryCollectorInterface { * @param entry - The telemetry log entry to record. */ recordLog(entry: TelemetryLogEntry): void { - if (!this.#shouldRecordLog(entry)) { - return; - } + try { + if (!this.#shouldRecordLog(entry)) { + return; + } - const levelIsValid = typeof entry?.level === 'string' && VALID_LOG_LEVELS.has(entry.level); - const messageIsValid = typeof entry?.message === 'string' && entry.message.trim().length > 0; + const levelIsValid = typeof entry?.level === 'string' && VALID_LOG_LEVELS.has(entry.level); + const messageIsValid = typeof entry?.message === 'string' && entry.message.trim().length > 0; - let normalizedTimestamp: Date | null = null; - const timestampInput: unknown = (entry as unknown as { timestamp?: unknown })?.timestamp; - if (typeof timestampInput === 'number' || typeof timestampInput === 'string') { - const candidate = new Date(timestampInput); - if (!Number.isNaN(candidate.getTime())) { - normalizedTimestamp = candidate; + let normalizedTimestamp: Date | null = null; + const timestampInput: unknown = (entry as unknown as { timestamp?: unknown })?.timestamp; + if (typeof timestampInput === 'number' || typeof timestampInput === 'string') { + const candidate = new Date(timestampInput); + if (!Number.isNaN(candidate.getTime())) { + normalizedTimestamp = candidate; + } } - } - if (!levelIsValid || !messageIsValid || normalizedTimestamp === null) { - if (this.isDebug && typeof console !== 'undefined') { - console.warn('[clerk/telemetry] Dropping invalid telemetry log entry', { - levelIsValid, - messageIsValid, - timestampIsValid: normalizedTimestamp !== null, - }); + if (!levelIsValid || !messageIsValid || normalizedTimestamp === null) { + if (this.isDebug && typeof console !== 'undefined') { + console.warn('[clerk/telemetry] Dropping invalid telemetry log entry', { + levelIsValid, + messageIsValid, + timestampIsValid: normalizedTimestamp !== null, + }); + } + return; } - return; - } - const sdkMetadata = this.#getSDKMetadata(); + const sdkMetadata = this.#getSDKMetadata(); - const logData: TelemetryLogData = { - sdk: sdkMetadata.name, - sdkv: sdkMetadata.version, - cv: this.#metadata.clerkVersion ?? '', - lvl: entry.level, - msg: entry.message, - ts: normalizedTimestamp.toISOString(), - pk: this.#metadata.publishableKey || null, - payload: this.#sanitizeContext(entry.context), - }; + const logData: TelemetryLogData = { + sdk: sdkMetadata.name, + sdkv: sdkMetadata.version, + cv: this.#metadata.clerkVersion ?? '', + lvl: entry.level, + msg: entry.message, + ts: normalizedTimestamp.toISOString(), + pk: this.#metadata.publishableKey || null, + payload: this.#sanitizeContext(entry.context), + }; - this.#buffer.push({ kind: 'log', value: logData }); + this.#buffer.push({ kind: 'log', value: logData }); - this.#scheduleFlush(); + this.#scheduleFlush(); + } catch (error) { + console.error('[clerk/telemetry] Error recording telemetry log entry', error); + } } #shouldRecord(preparedPayload: TelemetryEvent, eventSamplingRate?: number) { @@ -271,7 +279,6 @@ export class TelemetryCollector implements TelemetryCollectorInterface { this.#flush(); return; } - const isBufferFull = this.#buffer.length >= this.#config.maxBufferSize; if (isBufferFull) { // If the buffer is full, flush immediately to make sure we minimize the chance of event loss.