Skip to content
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

feat(replay): Clear event buffer when full and in buffer mode #14078

Merged
merged 16 commits into from
Nov 21, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@ export class EventBufferArray implements EventBuffer {
/** @inheritdoc */
public hasCheckout: boolean;

/** @inheritdoc */
public waitForCheckout: boolean;

private _totalSize: number;

public constructor() {
this.events = [];
this._totalSize = 0;
this.hasCheckout = false;
this.waitForCheckout = false;
}

/** @inheritdoc */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ export class EventBufferCompressionWorker implements EventBuffer {
/** @inheritdoc */
public hasCheckout: boolean;

/** @inheritdoc */
public waitForCheckout: boolean;

private _worker: WorkerHandler;
private _earliestTimestamp: number | null;
private _totalSize;
Expand All @@ -25,6 +28,7 @@ export class EventBufferCompressionWorker implements EventBuffer {
this._earliestTimestamp = null;
this._totalSize = 0;
this.hasCheckout = false;
this.waitForCheckout = false;
}

/** @inheritdoc */
Expand Down
14 changes: 13 additions & 1 deletion packages/replay-internal/src/eventBuffer/EventBufferProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export class EventBufferProxy implements EventBuffer {
this._ensureWorkerIsLoadedPromise = this._ensureWorkerIsLoaded();
}

/** @inheritdoc */
public get waitForCheckout(): boolean {
return this._used.waitForCheckout;
}

/** @inheritdoc */
public get type(): EventBufferType {
return this._used.type;
Expand All @@ -44,6 +49,12 @@ export class EventBufferProxy implements EventBuffer {
this._used.hasCheckout = value;
}

/** @inheritdoc */
// eslint-disable-next-line @typescript-eslint/adjacent-overload-signatures
public set waitForCheckout(value: boolean) {
this._used.waitForCheckout = value;
}

/** @inheritDoc */
public destroy(): void {
this._fallback.destroy();
Expand Down Expand Up @@ -99,14 +110,15 @@ export class EventBufferProxy implements EventBuffer {

/** Switch the used buffer to the compression worker. */
private async _switchToCompressionWorker(): Promise<void> {
const { events, hasCheckout } = this._fallback;
const { events, hasCheckout, waitForCheckout } = this._fallback;

const addEventPromises: Promise<void>[] = [];
for (const event of events) {
addEventPromises.push(this._compression.addEvent(event));
}

this._compression.hasCheckout = hasCheckout;
this._compression.waitForCheckout = waitForCheckout;

// We switch over to the new buffer immediately - any further events will be added
// after the previously buffered ones
Expand Down
6 changes: 6 additions & 0 deletions packages/replay-internal/src/types/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,12 @@ export interface EventBuffer {
*/
hasCheckout: boolean;

/**
* If the event buffer needs to wait for a checkout event before it
* starts buffering events.
*/
waitForCheckout: boolean;

/**
* Destroy the event buffer.
*/
Expand Down
27 changes: 21 additions & 6 deletions packages/replay-internal/src/util/addEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,22 @@ async function _addEvent(
event: RecordingEvent,
isCheckout?: boolean,
): Promise<AddEventResult | null> {
if (!replay.eventBuffer) {
const { eventBuffer } = replay;

if (!eventBuffer || (eventBuffer.waitForCheckout && !isCheckout)) {
return null;
}

const isBufferMode = replay.recordingMode === 'buffer';

try {
if (isCheckout && replay.recordingMode === 'buffer') {
replay.eventBuffer.clear();
if (isCheckout && isBufferMode) {
eventBuffer.clear();
}

if (isCheckout) {
replay.eventBuffer.hasCheckout = true;
eventBuffer.hasCheckout = true;
eventBuffer.waitForCheckout = false;
}

const replayOptions = replay.getOptions();
Expand All @@ -75,9 +80,19 @@ async function _addEvent(
return;
}

return await replay.eventBuffer.addEvent(eventAfterPossibleCallback);
return await eventBuffer.addEvent(eventAfterPossibleCallback);
} catch (error) {
const reason = error && error instanceof EventBufferSizeExceededError ? 'addEventSizeExceeded' : 'addEvent';
const isExceeded = error && error instanceof EventBufferSizeExceededError;
const reason = isExceeded ? 'addEventSizeExceeded' : 'addEvent';

if (isExceeded && isBufferMode) {
// Clear buffer and wait for next checkout
eventBuffer.clear();
eventBuffer.waitForCheckout = true;

return null;
}

replay.handleException(error);

await replay.stop({ reason });
Expand Down
75 changes: 75 additions & 0 deletions packages/replay-internal/test/integration/eventBuffer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* @vitest-environment jsdom
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { WINDOW } from '../../src/constants';
import type { Replay } from '../../src/integration';
import type { ReplayContainer } from '../../src/replay';
import { addEvent } from '../../src/util/addEvent';

// mock functions need to be imported first
import { BASE_TIMESTAMP, mockSdk } from '../index';
import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent';
import { useFakeTimers } from '../utils/use-fake-timers';

useFakeTimers();

describe('Integration | eventBuffer | Event Buffer Max Size', () => {
let replay: ReplayContainer;
let integration: Replay;
const prevLocation = WINDOW.location;

beforeEach(async () => {
vi.setSystemTime(new Date(BASE_TIMESTAMP));

({ replay, integration } = await mockSdk());

await vi.runAllTimersAsync();
vi.clearAllMocks();
});

afterEach(async () => {
vi.setSystemTime(new Date(BASE_TIMESTAMP));
integration && (await integration.stop());
Object.defineProperty(WINDOW, 'location', {
value: prevLocation,
writable: true,
});
vi.clearAllMocks();
});

it('does not add replay breadcrumb when stopped due to event buffer limit', async () => {
const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });

vi.mock('../../src/constants', async requireActual => ({
...(await requireActual<any>()),
REPLAY_MAX_EVENT_BUFFER_SIZE: 500,
}));

await integration.stop();
integration.startBuffering();

await addEvent(replay, TEST_EVENT);

expect(replay.eventBuffer?.hasEvents).toBe(true);
expect(replay.eventBuffer?.['hasCheckout']).toBe(true);

// This should should go over max buffer size
await addEvent(replay, TEST_EVENT);
// buffer should be cleared and wait for next checkout
expect(replay.eventBuffer?.hasEvents).toBe(false);
expect(replay.eventBuffer?.['hasCheckout']).toBe(false);

await addEvent(replay, TEST_EVENT);
expect(replay.eventBuffer?.hasEvents).toBe(false);
expect(replay.eventBuffer?.['hasCheckout']).toBe(false);

await addEvent(replay, getTestEventCheckout({ timestamp: Date.now() }), true);
expect(replay.eventBuffer?.hasEvents).toBe(true);
expect(replay.eventBuffer?.['hasCheckout']).toBe(true);

vi.resetAllMocks();
});
});
38 changes: 12 additions & 26 deletions packages/replay-internal/test/integration/stop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
*/

import type { MockInstance, MockedFunction } from 'vitest';
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import * as SentryBrowserUtils from '@sentry-internal/browser-utils';

import { WINDOW } from '../../src/constants';
import type { Replay } from '../../src/integration';
import type { ReplayContainer } from '../../src/replay';
import { clearSession } from '../../src/session/clearSession';
import { addEvent } from '../../src/util/addEvent';
import { createOptionsEvent } from '../../src/util/handleRecordingEmit';
// mock functions need to be imported first
Expand All @@ -32,7 +31,7 @@ describe('Integration | stop', () => {
let mockAddDomInstrumentationHandler: MockInstance;
let mockRunFlush: MockRunFlush;

beforeAll(async () => {
beforeEach(async () => {
vi.setSystemTime(new Date(BASE_TIMESTAMP));
mockAddDomInstrumentationHandler = vi.spyOn(SentryBrowserUtils, 'addClickKeypressInstrumentationHandler');

Expand All @@ -41,33 +40,18 @@ describe('Integration | stop', () => {
// @ts-expect-error private API
mockRunFlush = vi.spyOn(replay, '_runFlush');

vi.runAllTimers();
});

beforeEach(() => {
vi.setSystemTime(new Date(BASE_TIMESTAMP));
replay.eventBuffer?.destroy();
await vi.runAllTimersAsync();
vi.clearAllMocks();
});

afterEach(async () => {
vi.runAllTimers();
await new Promise(process.nextTick);
vi.setSystemTime(new Date(BASE_TIMESTAMP));
sessionStorage.clear();
clearSession(replay);
replay['_initializeSessionForSampling']();
replay.setInitialState();
mockRecord.takeFullSnapshot.mockClear();
mockAddDomInstrumentationHandler.mockClear();
integration && (await integration.stop());
Object.defineProperty(WINDOW, 'location', {
value: prevLocation,
writable: true,
});
});

afterAll(() => {
integration && integration.stop();
vi.clearAllMocks();
});

it('does not upload replay if it was stopped and can resume replays afterwards', async () => {
Expand Down Expand Up @@ -106,7 +90,7 @@ describe('Integration | stop', () => {

vi.advanceTimersByTime(ELAPSED);

const timestamp = +new Date(BASE_TIMESTAMP + ELAPSED + ELAPSED) / 1000;
const timestamp = +new Date(BASE_TIMESTAMP + ELAPSED + ELAPSED + ELAPSED) / 1000;

const hiddenBreadcrumb = {
type: 5,
Expand All @@ -123,15 +107,15 @@ describe('Integration | stop', () => {

addEvent(replay, TEST_EVENT);
WINDOW.dispatchEvent(new Event('blur'));
vi.runAllTimers();
await vi.runAllTimersAsync();
await new Promise(process.nextTick);
expect(replay).toHaveLastSentReplay({
recordingPayloadHeader: { segment_id: 0 },
recordingData: JSON.stringify([
// This event happens when we call `replay.start`
{
data: { isCheckout: true },
timestamp: BASE_TIMESTAMP + ELAPSED,
timestamp: BASE_TIMESTAMP + ELAPSED + ELAPSED,
type: 2,
},
optionsEvent,
Expand All @@ -142,12 +126,14 @@ describe('Integration | stop', () => {

// Session's last activity is last updated when we call `setup()` and *NOT*
// when tab is blurred
expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED);
expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED + ELAPSED);
});

it('does not buffer new events after being stopped', async function () {
expect(replay.eventBuffer?.hasEvents).toBe(false);
expect(mockRunFlush).toHaveBeenCalledTimes(0);
const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
addEvent(replay, TEST_EVENT);
addEvent(replay, TEST_EVENT, true);
expect(replay.eventBuffer?.hasEvents).toBe(true);
expect(mockRunFlush).toHaveBeenCalledTimes(0);

Expand Down
Loading