Skip to content

ref(replay): Explicitly clear eventBuffer for checkout events #7989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions packages/replay/src/eventBuffer/EventBufferArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,8 @@ export class EventBufferArray implements EventBuffer {
}

/** @inheritdoc */
public async addEvent(event: RecordingEvent, isCheckout?: boolean): Promise<AddEventResult> {
if (isCheckout) {
this.events = [event];
return;
}

public async addEvent(event: RecordingEvent): Promise<AddEventResult> {
this.events.push(event);
return;
}

/** @inheritdoc */
Expand All @@ -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];
Expand Down
21 changes: 8 additions & 13 deletions packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AddEventResult> {
if (isCheckout) {
// This event is a checkout, make sure worker buffer is cleared before
// proceeding.
await this._clear();
}

public addEvent(event: RecordingEvent): Promise<AddEventResult> {
const timestamp = timestampToMs(event.timestamp);
if (!this._earliestTimestamp || timestamp < this._earliestTimestamp) {
this._earliestTimestamp = timestamp;
Expand All @@ -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;
Expand All @@ -86,10 +87,4 @@ export class EventBufferCompressionWorker implements EventBuffer {

return response;
}

/** Clear any pending events from the worker. */
private _clear(): Promise<void> {
this._earliestTimestamp = null;
return this._worker.postMessage('clear');
}
}
9 changes: 7 additions & 2 deletions packages/replay/src/eventBuffer/EventBufferProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -45,8 +50,8 @@ export class EventBufferProxy implements EventBuffer {
*
* Returns true if event was successfully added.
*/
public addEvent(event: RecordingEvent, isCheckout?: boolean): Promise<AddEventResult> {
return this._used.addEvent(event, isCheckout);
public addEvent(event: RecordingEvent): Promise<AddEventResult> {
return this._used.addEvent(event);
}

/** @inheritDoc */
Expand Down
8 changes: 6 additions & 2 deletions packages/replay/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AddEventResult>;
addEvent(event: RecordingEvent): Promise<AddEventResult>;

/**
* Clears and returns the contents of the buffer.
Expand Down
6 changes: 5 additions & 1 deletion packages/replay/src/util/addEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down