From 94963de4ac5611027b44fbe108baf11407be73d5 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 27 Apr 2023 14:33:21 +0200 Subject: [PATCH] ref(replay): Explicitly clear eventBuffer for checkout events --- .../src/eventBuffer/EventBufferArray.ts | 13 ++++++------ .../EventBufferCompressionWorker.ts | 21 +++++++------------ .../src/eventBuffer/EventBufferProxy.ts | 9 ++++++-- packages/replay/src/types.ts | 8 +++++-- packages/replay/src/util/addEvent.ts | 6 +++++- .../unit/eventBuffer/EventBufferArray.test.ts | 4 +++- .../EventBufferCompressionWorker.test.ts | 7 ++++--- 7 files changed, 39 insertions(+), 29 deletions(-) diff --git a/packages/replay/src/eventBuffer/EventBufferArray.ts b/packages/replay/src/eventBuffer/EventBufferArray.ts index 5315fe3a7a1a..42ce00aabbc6 100644 --- a/packages/replay/src/eventBuffer/EventBufferArray.ts +++ b/packages/replay/src/eventBuffer/EventBufferArray.ts @@ -24,14 +24,8 @@ export class EventBufferArray implements EventBuffer { } /** @inheritdoc */ - public async addEvent(event: RecordingEvent, isCheckout?: boolean): Promise { - if (isCheckout) { - this.events = [event]; - return; - } - + public async addEvent(event: RecordingEvent): Promise { this.events.push(event); - return; } /** @inheritdoc */ @@ -46,6 +40,11 @@ export class EventBufferArray implements EventBuffer { }); } + /** @inheritdoc */ + public clear(): void { + this.events = []; + } + /** @inheritdoc */ public getEarliestTimestamp(): number | null { const timestamp = this.events.map(event => event.timestamp).sort()[0]; diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts index 64d096661536..57ad449dac55 100644 --- a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts @@ -42,13 +42,7 @@ export class EventBufferCompressionWorker implements EventBuffer { * * Returns true if event was successfuly received and processed by worker. */ - public async addEvent(event: RecordingEvent, isCheckout?: boolean): Promise { - if (isCheckout) { - // This event is a checkout, make sure worker buffer is cleared before - // proceeding. - await this._clear(); - } - + public addEvent(event: RecordingEvent): Promise { const timestamp = timestampToMs(event.timestamp); if (!this._earliestTimestamp || timestamp < this._earliestTimestamp) { this._earliestTimestamp = timestamp; @@ -64,6 +58,13 @@ export class EventBufferCompressionWorker implements EventBuffer { return this._finishRequest(); } + /** @inheritdoc */ + public clear(): void { + this._earliestTimestamp = null; + // We do not wait on this, as we assume the order of messages is consistent for the worker + void this._worker.postMessage('clear'); + } + /** @inheritdoc */ public getEarliestTimestamp(): number | null { return this._earliestTimestamp; @@ -86,10 +87,4 @@ export class EventBufferCompressionWorker implements EventBuffer { return response; } - - /** Clear any pending events from the worker. */ - private _clear(): Promise { - this._earliestTimestamp = null; - return this._worker.postMessage('clear'); - } } diff --git a/packages/replay/src/eventBuffer/EventBufferProxy.ts b/packages/replay/src/eventBuffer/EventBufferProxy.ts index af862d6714b5..972fc9d58911 100644 --- a/packages/replay/src/eventBuffer/EventBufferProxy.ts +++ b/packages/replay/src/eventBuffer/EventBufferProxy.ts @@ -35,6 +35,11 @@ export class EventBufferProxy implements EventBuffer { this._compression.destroy(); } + /** @inheritdoc */ + public clear(): void { + return this._used.clear(); + } + /** @inheritdoc */ public getEarliestTimestamp(): number | null { return this._used.getEarliestTimestamp(); @@ -45,8 +50,8 @@ export class EventBufferProxy implements EventBuffer { * * Returns true if event was successfully added. */ - public addEvent(event: RecordingEvent, isCheckout?: boolean): Promise { - return this._used.addEvent(event, isCheckout); + public addEvent(event: RecordingEvent): Promise { + return this._used.addEvent(event); } /** @inheritDoc */ diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 14a9079e16e6..0d653f3a7817 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -446,13 +446,17 @@ export interface EventBuffer { */ destroy(): void; + /** + * Clear the event buffer. + */ + clear(): void; + /** * Add an event to the event buffer. - * `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`. * * Returns a promise that resolves if the event was successfully added, else rejects. */ - addEvent(event: RecordingEvent, isCheckout?: boolean): Promise; + addEvent(event: RecordingEvent): Promise; /** * Clears and returns the contents of the buffer. diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index af4afbe6ad76..8064515c3f87 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -34,7 +34,11 @@ export async function addEvent( } try { - return await replay.eventBuffer.addEvent(event, isCheckout); + if (isCheckout) { + replay.eventBuffer.clear(); + } + + return await replay.eventBuffer.addEvent(event); } catch (error) { __DEBUG_BUILD__ && logger.error(error); await replay.stop('addEvent'); diff --git a/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts b/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts index 36d0b5aa0e00..37827610e4cc 100644 --- a/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts +++ b/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts @@ -22,7 +22,9 @@ describe('Unit | eventBuffer | EventBufferArray', () => { buffer.addEvent(TEST_EVENT); buffer.addEvent(TEST_EVENT); - buffer.addEvent(TEST_EVENT, true); + // clear() is called by addEvent when isCheckout is true + buffer.clear(); + buffer.addEvent(TEST_EVENT); const result = await buffer.finish(); expect(result).toEqual(JSON.stringify([TEST_EVENT])); diff --git a/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts b/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts index 1e22716b553a..74819a2cdf75 100644 --- a/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts +++ b/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts @@ -42,8 +42,9 @@ describe('Unit | eventBuffer | EventBufferCompressionWorker', () => { await buffer.addEvent(TEST_EVENT); await buffer.addEvent(TEST_EVENT); - // This should clear previous buffer - await buffer.addEvent({ ...TEST_EVENT, type: 2 }, true); + // clear() is called by addEvent when isCheckout is true + buffer.clear(); + await buffer.addEvent({ ...TEST_EVENT, type: 2 }); const result = await buffer.finish(); expect(result).toBeInstanceOf(Uint8Array); @@ -135,7 +136,7 @@ describe('Unit | eventBuffer | EventBufferCompressionWorker', () => { // Ensure worker is ready await buffer.ensureWorkerIsLoaded(); - await buffer.addEvent({ data: { o: 1 }, timestamp: BASE_TIMESTAMP, type: 3 }, true); + await buffer.addEvent({ data: { o: 1 }, timestamp: BASE_TIMESTAMP, type: 3 }); await buffer.addEvent({ data: { o: 2 }, timestamp: BASE_TIMESTAMP, type: 3 }); // @ts-ignore Mock this private so it triggers an error