diff --git a/.changeset/twenty-poems-push.md b/.changeset/twenty-poems-push.md new file mode 100644 index 00000000000..d6cd2916061 --- /dev/null +++ b/.changeset/twenty-poems-push.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': patch +--- + +Fix a potential memory leak in `TelemetryCollector` diff --git a/packages/shared/src/__tests__/telemetry.test.ts b/packages/shared/src/__tests__/telemetry.test.ts index e2f1fad38be..6a73e8d8594 100644 --- a/packages/shared/src/__tests__/telemetry.test.ts +++ b/packages/shared/src/__tests__/telemetry.test.ts @@ -150,6 +150,38 @@ describe('TelemetryCollector', () => { expect(fetchSpy).toHaveBeenCalled(); }); + test('events are isolated between batches (no shared buffer reference)', () => { + const collector = new TelemetryCollector({ + maxBufferSize: 2, + publishableKey: TEST_PK, + }); + const capturedBatches: TelemetryEvent[] = []; + + fetchSpy.mockImplementation((_, options) => { + capturedBatches.push(JSON.parse(options.body).events); + return Promise.resolve({ ok: true }); + }); + + // First batch + collector.record({ event: 'A', payload: { id: 1 } }); + collector.record({ event: 'B', payload: { id: 2 } }); + + // Second batch + collector.record({ event: 'C', payload: { id: 3 } }); + collector.record({ event: 'D', payload: { id: 4 } }); + + // If there's a closure leak, events might be mixed or duplicated + expect(capturedBatches[0]).toEqual([ + expect.objectContaining({ event: 'A', payload: { id: 1 } }), + expect.objectContaining({ event: 'B', payload: { id: 2 } }), + ]); + + expect(capturedBatches[1]).toEqual([ + expect.objectContaining({ event: 'C', payload: { id: 3 } }), + expect.objectContaining({ event: 'D', payload: { id: 4 } }), + ]); + }); + describe('with server-side sampling', () => { test('does not send events if the random seed does not exceed the event-specific sampling rate', async () => { windowSpy.mockImplementation(() => undefined); diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 86df7cc60d9..638073968c2 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -213,21 +213,27 @@ export class TelemetryCollector implements TelemetryCollectorInterface { } #flush(): void { + // Capture the current buffer and clear it immediately to avoid closure references + const eventsToSend = [...this.#buffer]; + this.#buffer = []; + + this.#pendingFlush = null; + + if (eventsToSend.length === 0) { + return; + } + fetch(new URL('/v1/event', this.#config.endpoint), { method: 'POST', // TODO: We send an array here with that idea that we can eventually send multiple events. body: JSON.stringify({ - events: this.#buffer, + events: eventsToSend, }), + keepalive: true, headers: { 'Content-Type': 'application/json', }, - }) - .catch(() => void 0) - .then(() => { - this.#buffer = []; - }) - .catch(() => void 0); + }).catch(() => void 0); } /** diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 57aefcc0147..8258e0c6892 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -1,6 +1,7 @@ import type { TelemetryEventRaw } from '@clerk/types'; const EVENT_METHOD_CALLED = 'METHOD_CALLED'; +const EVENT_SAMPLING_RATE = 0.1; type EventMethodCalled = { method: string; @@ -15,6 +16,7 @@ export function eventMethodCalled( ): TelemetryEventRaw { return { event: EVENT_METHOD_CALLED, + eventSamplingRate: EVENT_SAMPLING_RATE, payload: { method, ...payload,