ref(replay): Improve handling of checkout events

Author: mydea

packages/integration-tests/suites/replay/multiple-pages/test.ts

@@ -45,21 +45,31 @@ sentryTest(
 
     const reqPromise0 = waitForReplayRequest(page, 0);
     const reqPromise1 = waitForReplayRequest(page, 1);
+    const reqPromise2 = waitForReplayRequest(page, 2);
+    const reqPromise3 = waitForReplayRequest(page, 3);
+    const reqPromise4 = waitForReplayRequest(page, 4);
+    const reqPromise5 = waitForReplayRequest(page, 5);
+    const reqPromise6 = waitForReplayRequest(page, 6);
+    const reqPromise7 = waitForReplayRequest(page, 7);
+    const reqPromise8 = waitForReplayRequest(page, 8);
+    const reqPromise9 = waitForReplayRequest(page, 9);
 
     const url = await getLocalTestPath({ testDir: __dirname });
 
     await page.goto(url);
-    const replayEvent0 = getReplayEvent(await reqPromise0);
-    const recording0 = getReplayRecordingContent(await reqPromise0);
+    const req0 = await reqPromise0;
+    const replayEvent0 = getReplayEvent(req0);
+    const recording0 = getReplayRecordingContent(req0);
 
     expect(replayEvent0).toEqual(getExpectedReplayEvent({ segment_id: 0 }));
     expect(normalize(recording0.fullSnapshots)).toMatchSnapshot('seg-0-snap-full');
     expect(recording0.incrementalSnapshots.length).toEqual(0);
 
     await page.click('#go-background');
 
-    const replayEvent1 = getReplayEvent(await reqPromise1);
-    const recording1 = getReplayRecordingContent(await reqPromise1);
+    const req1 = await reqPromise1;
+    const replayEvent1 = getReplayEvent(req1);
+    const recording1 = getReplayRecordingContent(req1);
 
     expect(replayEvent1).toEqual(
       getExpectedReplayEvent({ segment_id: 1, urls: [], replay_start_timestamp: undefined }),
@@ -91,20 +101,19 @@ sentryTest(
 
     await page.reload();
 
-    const reqPromise2 = waitForReplayRequest(page, 2);
-    const reqPromise3 = waitForReplayRequest(page, 3);
-
-    const replayEvent2 = getReplayEvent(await reqPromise2);
-    const recording2 = getReplayRecordingContent(await reqPromise2);
+    const req2 = await reqPromise2;
+    const replayEvent2 = getReplayEvent(req2);
+    const recording2 = getReplayRecordingContent(req2);
 
     expect(replayEvent2).toEqual(getExpectedReplayEvent({ segment_id: 2, replay_start_timestamp: undefined }));
     expect(normalize(recording2.fullSnapshots)).toMatchSnapshot('seg-2-snap-full');
     expect(recording2.incrementalSnapshots.length).toEqual(0);
 
     await page.click('#go-background');
 
-    const replayEvent3 = getReplayEvent(await reqPromise3);
-    const recording3 = getReplayRecordingContent(await reqPromise3);
+    const req3 = await reqPromise3;
+    const replayEvent3 = getReplayEvent(req3);
+    const recording3 = getReplayRecordingContent(req3);
 
     expect(replayEvent3).toEqual(
       getExpectedReplayEvent({ segment_id: 3, urls: [], replay_start_timestamp: undefined }),
@@ -134,11 +143,9 @@ sentryTest(
 
     await page.click('a');
 
-    const reqPromise4 = waitForReplayRequest(page, 4);
-    const reqPromise5 = waitForReplayRequest(page, 5);
-
-    const replayEvent4 = getReplayEvent(await reqPromise4);
-    const recording4 = getReplayRecordingContent(await reqPromise4);
+    const req4 = await reqPromise4;
+    const replayEvent4 = getReplayEvent(req4);
+    const recording4 = getReplayRecordingContent(req4);
 
     expect(replayEvent4).toEqual(
       getExpectedReplayEvent({
@@ -161,8 +168,9 @@ sentryTest(
 
     await page.click('#go-background');
 
-    const replayEvent5 = getReplayEvent(await reqPromise5);
-    const recording5 = getReplayRecordingContent(await reqPromise5);
+    const req5 = await reqPromise5;
+    const replayEvent5 = getReplayEvent(req5);
+    const recording5 = getReplayRecordingContent(req5);
 
     expect(replayEvent5).toEqual(
       getExpectedReplayEvent({
@@ -207,9 +215,9 @@ sentryTest(
 
     await page.click('#spa-navigation');
 
-    const reqPromise6 = waitForReplayRequest(page, 6);
-    const replayEvent6 = getReplayEvent(await reqPromise6);
-    const recording6 = getReplayRecordingContent(await reqPromise6);
+    const req6 = await reqPromise6;
+    const replayEvent6 = getReplayEvent(req6);
+    const recording6 = getReplayRecordingContent(req6);
 
     expect(replayEvent6).toEqual(
       getExpectedReplayEvent({
@@ -231,9 +239,9 @@ sentryTest(
 
     await page.click('#go-background');
 
-    const reqPromise7 = waitForReplayRequest(page, 7);
-    const replayEvent7 = getReplayEvent(await reqPromise7);
-    const recording7 = getReplayRecordingContent(await reqPromise7);
+    const req7 = await reqPromise7;
+    const replayEvent7 = getReplayEvent(req7);
+    const recording7 = getReplayRecordingContent(req7);
 
     expect(replayEvent7).toEqual(
       getExpectedReplayEvent({
@@ -279,11 +287,9 @@ sentryTest(
 
     await page.click('a');
 
-    const reqPromise8 = waitForReplayRequest(page, 8);
-    const reqPromise9 = waitForReplayRequest(page, 9);
-
-    const replayEvent8 = getReplayEvent(await reqPromise8);
-    const recording8 = getReplayRecordingContent(await reqPromise8);
+    const req8 = await reqPromise8;
+    const replayEvent8 = getReplayEvent(req8);
+    const recording8 = getReplayRecordingContent(req8);
 
     expect(replayEvent8).toEqual(
       getExpectedReplayEvent({
@@ -296,8 +302,9 @@ sentryTest(
 
     await page.click('#go-background');
 
-    const replayEvent9 = getReplayEvent(await reqPromise9);
-    const recording9 = getReplayRecordingContent(await reqPromise9);
+    const req9 = await reqPromise9;
+    const replayEvent9 = getReplayEvent(req9);
+    const recording9 = getReplayRecordingContent(req9);
 
     expect(replayEvent9).toEqual(
       getExpectedReplayEvent({

packages/integration-tests/suites/replay/multiple-pages/test.ts-snapshots/seg-6-snap-incremental

@@ -1,15 +1,4 @@
 [
-  {
-    "source": 1,
-    "positions": [
-      {
-        "x": 157,
-        "y": 90,
-        "id": 15,
-        "timeOffset": [timeOffset]
-      }
-    ]
-  },
   {
     "source": 2,
     "type": 1,
@@ -40,5 +29,16 @@
     "id": 15,
     "x": 157,
     "y": 90
+  },
+  {
+    "source": 1,
+    "positions": [
+      {
+        "x": 157,
+        "y": 90,
+        "id": 15,
+        "timeOffset": [timeOffset]
+      }
+    ]
   }
 ]

packages/integration-tests/suites/replay/multiple-pages/test.ts-snapshots/seg-6-snap-incremental-chromium

@@ -1,15 +1,4 @@
 [
-  {
-    "source": 1,
-    "positions": [
-      {
-        "x": 157,
-        "y": 90,
-        "id": 15,
-        "timeOffset": [timeOffset]
-      }
-    ]
-  },
   {
     "source": 2,
     "type": 1,
@@ -40,5 +29,16 @@
     "id": 15,
     "x": 157,
     "y": 90
+  },
+  {
+    "source": 1,
+    "positions": [
+      {
+        "x": 157,
+        "y": 90,
+        "id": 15,
+        "timeOffset": [timeOffset]
+      }
+    ]
   }
 ]

packages/replay/src/types.ts

@@ -277,6 +277,7 @@ export interface EventBuffer {
 
   /**
    * 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.
    */

packages/replay/src/util/addEvent.ts

@@ -4,7 +4,8 @@ import { logger } from '@sentry/utils';
 import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types';
 
 /**
- * Add an event to the event buffer
+ * Add an event to the event buffer.
+ * `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`.
  */
 export async function addEvent(
   replay: ReplayContainer,

packages/replay/src/util/handleRecordingEmit.ts

@@ -1,8 +1,7 @@
 import { logger } from '@sentry/utils';
 
-import type { ReplayContainer } from '../replay';
 import { saveSession } from '../session/saveSession';
-import type { RecordingEvent } from '../types';
+import type { RecordingEvent, ReplayContainer } from '../types';
 import { addEvent } from './addEvent';
 
 type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => void;
@@ -13,21 +12,29 @@ type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => vo
  * Adds to event buffer, and has varying flushing behaviors if the event was a checkout.
  */
 export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCallback {
-  return (event: RecordingEvent, isCheckout?: boolean) => {
+  let hadFirstEvent = false;
+
+  return (event: RecordingEvent, _isCheckout?: boolean) => {
     // If this is false, it means session is expired, create and a new session and wait for checkout
     if (!replay.checkAndHandleExpiredSession()) {
       __DEBUG_BUILD__ && logger.warn('[Replay] Received replay event after session expired.');
 
       return;
     }
 
+    // `_isCheckout` is only set when the checkout is due to `checkoutEveryNms`
+    // We also want to treat the first event as a checkout, so we handle this specifically here
+    const isCheckout = _isCheckout || !hadFirstEvent;
+    hadFirstEvent = true;
+
+    // The handler returns `true` if we do not want to trigger debounced flush, `false` if we want to debounce flush.
     replay.addUpdate(() => {
       // The session is always started immediately on pageload/init, but for
       // error-only replays, it should reflect the most recent checkout
       // when an error occurs. Clear any state that happens before this current
       // checkout. This needs to happen before `addEvent()` which updates state
       // dependent on this reset.
-      if (replay.recordingMode === 'error' && event.type === 2) {
+      if (replay.recordingMode === 'error' && isCheckout) {
         replay.setInitialState();
       }
 
@@ -37,7 +44,7 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
 
       // Different behavior for full snapshots (type=2), ignore other event types
       // See https://github.com/rrweb-io/rrweb/blob/d8f9290ca496712aa1e7d472549480c4e7876594/packages/rrweb/src/types.ts#L16
-      if (event.type !== 2) {
+      if (!isCheckout) {
         return false;
       }
 

packages/replay/test/unit/util/handleRecordingEmit.test.ts

@@ -0,0 +1,86 @@
+import { EventType } from '@sentry-internal/rrweb';
+
+import { BASE_TIMESTAMP } from '../..';
+import * as SentryAddEvent from '../../../src/util/addEvent';
+import { getHandleRecordingEmit } from '../../../src/util/handleRecordingEmit';
+import { setupReplayContainer } from '../../utils/setupReplayContainer';
+import { useFakeTimers } from '../../utils/use-fake-timers';
+
+useFakeTimers();
+
+describe('Unit | util | handleRecordingEmit', () => {
+  let addEventMock: jest.SpyInstance;
+
+  beforeEach(function () {
+    jest.setSystemTime(BASE_TIMESTAMP);
+    addEventMock = jest.spyOn(SentryAddEvent, 'addEvent').mockImplementation(async () => {
+      // Do nothing
+    });
+  });
+
+  afterEach(function () {
+    addEventMock.mockReset();
+  });
+
+  it('interprets first event as checkout event', async function () {
+    const replay = setupReplayContainer({
+      options: {
+        errorSampleRate: 0,
+        sessionSampleRate: 1,
+      },
+    });
+
+    const handler = getHandleRecordingEmit(replay);
+
+    const event = {
+      type: EventType.FullSnapshot,
+      data: {
+        tag: 'test custom',
+      },
+      timestamp: BASE_TIMESTAMP + 10,
+    };
+
+    handler(event);
+    await new Promise(process.nextTick);
+
+    expect(addEventMock).toBeCalledTimes(1);
+    expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true);
+
+    handler(event);
+    await new Promise(process.nextTick);
+
+    expect(addEventMock).toBeCalledTimes(2);
+    expect(addEventMock).toHaveBeenLastCalledWith(replay, event, false);
+  });
+
+  it('interprets any event with isCheckout as checkout', async function () {
+    const replay = setupReplayContainer({
+      options: {
+        errorSampleRate: 0,
+        sessionSampleRate: 1,
+      },
+    });
+
+    const handler = getHandleRecordingEmit(replay);
+
+    const event = {
+      type: EventType.IncrementalSnapshot,
+      data: {
+        tag: 'test custom',
+      },
+      timestamp: BASE_TIMESTAMP + 10,
+    };
+
+    handler(event, true);
+    await new Promise(process.nextTick);
+
+    expect(addEventMock).toBeCalledTimes(1);
+    expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true);
+
+    handler(event, true);
+    await new Promise(process.nextTick);
+
+    expect(addEventMock).toBeCalledTimes(2);
+    expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true);
+  });
+});