Skip to content

Commit 05b6d65

Browse files
jacekradkojacekradko
authored
feat(shared): Do not allow telemetry errors to bubble up (#6640)
1 parent 2746cdb commit 05b6d65

File tree

4 files changed

+195
-49
lines changed

4 files changed

+195
-49
lines changed

‎.changeset/thirty-cars-beg.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/shared': patch
3+
---
4+
5+
Do not allow errors in telemetry to bubble up

‎packages/shared/src/__tests__/telemetry.logs.test.ts‎

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ describe('TelemetryCollector.recordLog', () => {
4949
expect(log.iid).toBeUndefined();
5050
expect(log.ts).toBe(new Date(ts).toISOString());
5151
expect(log.pk).toBe(TEST_PK);
52-
// Function and undefined stripped out
5352
expect(log.payload).toEqual({ a: 1 });
5453
});
5554

@@ -62,7 +61,6 @@ describe('TelemetryCollector.recordLog', () => {
6261
timestamp: Date.now(),
6362
};
6463

65-
// undefined context
6664
fetchSpy.mockClear();
6765
collector.recordLog({ ...base, context: undefined } as any);
6866
jest.runAllTimers();
@@ -71,7 +69,6 @@ describe('TelemetryCollector.recordLog', () => {
7169
let body = JSON.parse(initOptions1.body as string);
7270
expect(body.logs[0].payload).toBeNull();
7371

74-
// array context
7572
fetchSpy.mockClear();
7673
collector.recordLog({ ...base, context: [1, 2, 3] } as any);
7774
jest.runAllTimers();
@@ -80,7 +77,6 @@ describe('TelemetryCollector.recordLog', () => {
8077
body = JSON.parse(initOptions2.body as string);
8178
expect(body.logs[0].payload).toBeNull();
8279

83-
// circular context
8480
fetchSpy.mockClear();
8581
const circular: any = { foo: 'bar' };
8682
circular.self = circular;
@@ -95,7 +91,6 @@ describe('TelemetryCollector.recordLog', () => {
9591
test('drops invalid entries: missing id, invalid level, empty message, invalid timestamp', () => {
9692
const collector = new TelemetryCollector({ publishableKey: TEST_PK });
9793

98-
// invalid level
9994
fetchSpy.mockClear();
10095
collector.recordLog({
10196
level: 'fatal' as unknown as any,
@@ -105,7 +100,6 @@ describe('TelemetryCollector.recordLog', () => {
105100
jest.runAllTimers();
106101
expect(fetchSpy).not.toHaveBeenCalled();
107102

108-
// empty message
109103
fetchSpy.mockClear();
110104
collector.recordLog({
111105
level: 'debug',
@@ -115,7 +109,6 @@ describe('TelemetryCollector.recordLog', () => {
115109
jest.runAllTimers();
116110
expect(fetchSpy).not.toHaveBeenCalled();
117111

118-
// invalid timestamp (NaN)
119112
fetchSpy.mockClear();
120113
collector.recordLog({
121114
level: 'warn',
@@ -144,4 +137,86 @@ describe('TelemetryCollector.recordLog', () => {
144137
const body = JSON.parse(initOptions4.body as string);
145138
expect(body.logs[0].ts).toBe(new Date(tsString).toISOString());
146139
});
140+
141+
describe('error handling', () => {
142+
test('recordLog() method handles circular references in context gracefully', () => {
143+
const collector = new TelemetryCollector({ publishableKey: TEST_PK });
144+
145+
const circularContext = (() => {
146+
const obj: any = { test: 'value' };
147+
obj.self = obj;
148+
return obj;
149+
})();
150+
151+
expect(() => {
152+
collector.recordLog({
153+
level: 'info',
154+
message: 'test message',
155+
timestamp: Date.now(),
156+
context: circularContext,
157+
});
158+
}).not.toThrow();
159+
160+
jest.runAllTimers();
161+
expect(fetchSpy).toHaveBeenCalled();
162+
163+
const [url, init] = fetchSpy.mock.calls[0];
164+
expect(String(url)).toMatch('/v1/logs');
165+
166+
const initOptions = init as RequestInit;
167+
const body = JSON.parse(initOptions.body as string);
168+
expect(body.logs[0].payload).toBeNull();
169+
});
170+
171+
test('recordLog() method handles non-serializable context gracefully', () => {
172+
const collector = new TelemetryCollector({ publishableKey: TEST_PK });
173+
174+
const nonSerializableContext = {
175+
function: () => 'test',
176+
undefined: undefined,
177+
symbol: Symbol('test'),
178+
circular: (() => {
179+
const obj: any = { test: 'value' };
180+
obj.self = obj;
181+
return obj;
182+
})(),
183+
};
184+
185+
expect(() => {
186+
collector.recordLog({
187+
level: 'info',
188+
message: 'test message',
189+
timestamp: Date.now(),
190+
context: nonSerializableContext,
191+
});
192+
}).not.toThrow();
193+
194+
jest.runAllTimers();
195+
expect(fetchSpy).toHaveBeenCalled();
196+
197+
const [url, init] = fetchSpy.mock.calls[0];
198+
expect(String(url)).toMatch('/v1/logs');
199+
200+
const initOptions = init as RequestInit;
201+
const body = JSON.parse(initOptions.body as string);
202+
expect(body.logs[0].payload).toBeNull();
203+
});
204+
205+
test('recordLog() method handles invalid timestamp gracefully', () => {
206+
const collector = new TelemetryCollector({ publishableKey: TEST_PK });
207+
208+
const invalidTimestamp = new Date('invalid date');
209+
210+
expect(() => {
211+
collector.recordLog({
212+
level: 'info',
213+
message: 'test message',
214+
timestamp: invalidTimestamp.getTime(),
215+
});
216+
}).not.toThrow();
217+
218+
jest.runAllTimers();
219+
expect(fetchSpy).not.toHaveBeenCalled();
220+
});
221+
});
147222
});

‎packages/shared/src/__tests__/telemetry.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,4 +392,63 @@ describe('TelemetryCollector', () => {
392392
fetchSpy.mockRestore();
393393
});
394394
});
395+
396+
describe('error handling', () => {
397+
test('record() method does not bubble up errors from internal operations', () => {
398+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
399+
400+
const collector = new TelemetryCollector({
401+
publishableKey: TEST_PK,
402+
});
403+
404+
const circularPayload = (() => {
405+
const obj: any = { test: 'value' };
406+
obj.self = obj;
407+
return obj;
408+
})();
409+
410+
expect(() => {
411+
collector.record({ event: 'TEST_EVENT', payload: circularPayload });
412+
}).not.toThrow();
413+
414+
expect(consoleErrorSpy).toHaveBeenCalledWith(
415+
'[clerk/telemetry] Error recording telemetry event',
416+
expect.any(Error),
417+
);
418+
419+
consoleErrorSpy.mockRestore();
420+
});
421+
422+
test('record() method handles errors gracefully and continues operation', () => {
423+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
424+
425+
const collector = new TelemetryCollector({
426+
publishableKey: TEST_PK,
427+
});
428+
429+
const problematicPayload = (() => {
430+
const obj: any = { test: 'value' };
431+
obj.self = obj;
432+
return obj;
433+
})();
434+
435+
expect(() => {
436+
collector.record({ event: 'TEST_EVENT', payload: problematicPayload });
437+
}).not.toThrow();
438+
439+
expect(consoleErrorSpy).toHaveBeenCalledWith(
440+
'[clerk/telemetry] Error recording telemetry event',
441+
expect.any(Error),
442+
);
443+
444+
expect(() => {
445+
collector.record({ event: 'TEST_EVENT', payload: { normal: 'data' } });
446+
}).not.toThrow();
447+
448+
jest.runAllTimers();
449+
expect(fetchSpy).toHaveBeenCalled();
450+
451+
consoleErrorSpy.mockRestore();
452+
});
453+
});
395454
});

‎packages/shared/src/telemetry/collector.ts‎

Lines changed: 49 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -176,17 +176,21 @@ export class TelemetryCollector implements TelemetryCollectorInterface {
176176
}
177177

178178
record(event: TelemetryEventRaw): void {
179-
const preparedPayload = this.#preparePayload(event.event, event.payload);
179+
try {
180+
const preparedPayload = this.#preparePayload(event.event, event.payload);
180181

181-
this.#logEvent(preparedPayload.event, preparedPayload);
182+
this.#logEvent(preparedPayload.event, preparedPayload);
182183

183-
if (!this.#shouldRecord(preparedPayload, event.eventSamplingRate)) {
184-
return;
185-
}
184+
if (!this.#shouldRecord(preparedPayload, event.eventSamplingRate)) {
185+
return;
186+
}
186187

187-
this.#buffer.push({ kind: 'event', value: preparedPayload });
188+
this.#buffer.push({ kind: 'event', value: preparedPayload });
188189

189-
this.#scheduleFlush();
190+
this.#scheduleFlush();
191+
} catch (error) {
192+
console.error('[clerk/telemetry] Error recording telemetry event', error);
193+
}
190194
}
191195

192196
/**
@@ -195,49 +199,53 @@ export class TelemetryCollector implements TelemetryCollectorInterface {
195199
* @param entry - The telemetry log entry to record.
196200
*/
197201
recordLog(entry: TelemetryLogEntry): void {
198-
if (!this.#shouldRecordLog(entry)) {
199-
return;
200-
}
202+
try {
203+
if (!this.#shouldRecordLog(entry)) {
204+
return;
205+
}
201206

202-
const levelIsValid = typeof entry?.level === 'string' && VALID_LOG_LEVELS.has(entry.level);
203-
const messageIsValid = typeof entry?.message === 'string' && entry.message.trim().length > 0;
207+
const levelIsValid = typeof entry?.level === 'string' && VALID_LOG_LEVELS.has(entry.level);
208+
const messageIsValid = typeof entry?.message === 'string' && entry.message.trim().length > 0;
204209

205-
let normalizedTimestamp: Date | null = null;
206-
const timestampInput: unknown = (entry as unknown as { timestamp?: unknown })?.timestamp;
207-
if (typeof timestampInput === 'number' || typeof timestampInput === 'string') {
208-
const candidate = new Date(timestampInput);
209-
if (!Number.isNaN(candidate.getTime())) {
210-
normalizedTimestamp = candidate;
210+
let normalizedTimestamp: Date | null = null;
211+
const timestampInput: unknown = (entry as unknown as { timestamp?: unknown })?.timestamp;
212+
if (typeof timestampInput === 'number' || typeof timestampInput === 'string') {
213+
const candidate = new Date(timestampInput);
214+
if (!Number.isNaN(candidate.getTime())) {
215+
normalizedTimestamp = candidate;
216+
}
211217
}
212-
}
213218

214-
if (!levelIsValid || !messageIsValid || normalizedTimestamp === null) {
215-
if (this.isDebug && typeof console !== 'undefined') {
216-
console.warn('[clerk/telemetry] Dropping invalid telemetry log entry', {
217-
levelIsValid,
218-
messageIsValid,
219-
timestampIsValid: normalizedTimestamp !== null,
220-
});
219+
if (!levelIsValid || !messageIsValid || normalizedTimestamp === null) {
220+
if (this.isDebug && typeof console !== 'undefined') {
221+
console.warn('[clerk/telemetry] Dropping invalid telemetry log entry', {
222+
levelIsValid,
223+
messageIsValid,
224+
timestampIsValid: normalizedTimestamp !== null,
225+
});
226+
}
227+
return;
221228
}
222-
return;
223-
}
224229

225-
const sdkMetadata = this.#getSDKMetadata();
230+
const sdkMetadata = this.#getSDKMetadata();
226231

227-
const logData: TelemetryLogData = {
228-
sdk: sdkMetadata.name,
229-
sdkv: sdkMetadata.version,
230-
cv: this.#metadata.clerkVersion ?? '',
231-
lvl: entry.level,
232-
msg: entry.message,
233-
ts: normalizedTimestamp.toISOString(),
234-
pk: this.#metadata.publishableKey || null,
235-
payload: this.#sanitizeContext(entry.context),
236-
};
232+
const logData: TelemetryLogData = {
233+
sdk: sdkMetadata.name,
234+
sdkv: sdkMetadata.version,
235+
cv: this.#metadata.clerkVersion ?? '',
236+
lvl: entry.level,
237+
msg: entry.message,
238+
ts: normalizedTimestamp.toISOString(),
239+
pk: this.#metadata.publishableKey || null,
240+
payload: this.#sanitizeContext(entry.context),
241+
};
237242

238-
this.#buffer.push({ kind: 'log', value: logData });
243+
this.#buffer.push({ kind: 'log', value: logData });
239244

240-
this.#scheduleFlush();
245+
this.#scheduleFlush();
246+
} catch (error) {
247+
console.error('[clerk/telemetry] Error recording telemetry log entry', error);
248+
}
241249
}
242250

243251
#shouldRecord(preparedPayload: TelemetryEvent, eventSamplingRate?: number) {
@@ -271,7 +279,6 @@ export class TelemetryCollector implements TelemetryCollectorInterface {
271279
this.#flush();
272280
return;
273281
}
274-
275282
const isBufferFull = this.#buffer.length >= this.#config.maxBufferSize;
276283
if (isBufferFull) {
277284
// If the buffer is full, flush immediately to make sure we minimize the chance of event loss.

0 commit comments

Comments
 (0)