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

This makes a change so that when the event buffer is full, we will clear it and wait for the next checkout instead of erroring and stopping the replay buffering. This does mean that it's possible an exception occurs in-between the filled buffer and the next checkout, where we will have an empty replay.

Author: billyvg

packages/replay-internal/src/eventBuffer/EventBufferArray.ts

@@ -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 */

packages/replay-internal/src/eventBuffer/EventBufferCompressionWorker.ts

@@ -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;
@@ -25,6 +28,7 @@ export class EventBufferCompressionWorker implements EventBuffer {
     this._earliestTimestamp = null;
     this._totalSize = 0;
     this.hasCheckout = false;
+    this.waitForCheckout = false;
   }
 
   /** @inheritdoc */

packages/replay-internal/src/eventBuffer/EventBufferProxy.ts

@@ -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;
@@ -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();
@@ -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

packages/replay-internal/src/types/replay.ts

@@ -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.
    */

packages/replay-internal/src/util/addEvent.ts

@@ -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();
@@ -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 });

packages/replay-internal/test/integration/eventBuffer.test.ts

@@ -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();
+  });
+});

packages/replay-internal/test/integration/stop.test.ts

@@ -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
@@ -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');
 
@@ -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 () => {
@@ -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,
@@ -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,
@@ -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);