feat(replay): Do not capture replays < 5 seconds

Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab.

Closes getsentry/team-replay#63

Author: billyvg

packages/replay/src/replay.ts

@@ -462,7 +462,17 @@ export class ReplayContainer implements ReplayContainerInterface {
   }
 
   /**
-   *
+   * Only flush if `this.recordingMode === 'session'`
+   */
+  public conditionalFlush(): Promise<void> {
+    if (this.recordingMode === 'buffer') {
+      return Promise.resolve();
+    }
+
+    return this.flushImmediate();
+  }
+
+  /**
    * Always flush via `_debouncedFlush` so that we do not have flushes triggered
    * from calling both `flush` and `_debouncedFlush`. Otherwise, there could be
    * cases of mulitple flushes happening closely together.
@@ -473,6 +483,13 @@ export class ReplayContainer implements ReplayContainerInterface {
     return this._debouncedFlush.flush() as Promise<void>;
   }
 
+  /**
+   * Cancels queued up flushes.
+   */
+  public cancelFlush(): void {
+    this._debouncedFlush.cancel();
+  }
+
   /** Get the current sesion (=replay) ID */
   public getSessionId(): string | undefined {
     return this.session && this.session.id;
@@ -715,7 +732,7 @@ export class ReplayContainer implements ReplayContainerInterface {
     // Send replay when the page/tab becomes hidden. There is no reason to send
     // replay if it becomes visible, since no actions we care about were done
     // while it was hidden
-    this._conditionalFlush();
+    void this.conditionalFlush();
   }
 
   /**
@@ -799,17 +816,6 @@ export class ReplayContainer implements ReplayContainerInterface {
     return Promise.all(createPerformanceSpans(this, createPerformanceEntries(entries)));
   }
 
-  /**
-   * Only flush if `this.recordingMode === 'session'`
-   */
-  private _conditionalFlush(): void {
-    if (this.recordingMode === 'buffer') {
-      return;
-    }
-
-    void this.flushImmediate();
-  }
-
   /**
    * Clear _context
    */

packages/replay/src/types.ts

@@ -488,7 +488,9 @@ export interface ReplayContainer {
   startRecording(): void;
   stopRecording(): boolean;
   sendBufferedReplayOrFlush(options?: SendBufferedReplayOptions): Promise<void>;
+  conditionalFlush(): Promise<void>;
   flushImmediate(): Promise<void>;
+  cancelFlush(): void;
   triggerUserActivity(): void;
   addUpdate(cb: AddUpdateCallback): void;
   getOptions(): ReplayPluginOptions;

packages/replay/src/util/handleRecordingEmit.ts

@@ -75,10 +75,26 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
       // it can prevent loading on the UI. This will cause an increase in short
       // replays (e.g. opening and closing a tab quickly), but these can be
       // filtered on the UI.
-      if (replay.recordingMode === 'session') {
-        // We want to ensure the worker is ready, as otherwise we'd always send the first event uncompressed
-        void replay.flushImmediate();
-      }
+      // if (replay.recordingMode === 'session') {
+      //   // We want to ensure the worker is ready, as otherwise we'd always send the first event uncompressed
+      //   void replay.flushImmediate();
+      // }
+
+      // If the full snapshot is due to an initial load, we will not have
+      // a previous session ID. In this case, we want to buffer events
+      // for a set amount of time before flushing. This can help avoid
+      // capturing replays of users that immediately close the window.
+      setTimeout(() => replay.conditionalFlush(), replay.getOptions().flushMinDelay);
+
+      // Cancel any previously debounced flushes to ensure there are no [near]
+      // simultaneous flushes happening. The latter request should be
+      // insignificant in this case, so wait for additional user interaction to
+      // trigger a new flush.
+      //
+      // This can happen because there's no guarantee that a recording event
+      // happens first. e.g. a mouse click can happen and trigger a debounced
+      // flush before the checkout.
+      replay.cancelFlush();
 
       return true;
     });

packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts

@@ -1,7 +1,7 @@
 import { getCurrentHub } from '@sentry/core';
 import type { ErrorEvent, Event } from '@sentry/types';
 
-import { UNABLE_TO_SEND_REPLAY } from '../../../src/constants';
+import { UNABLE_TO_SEND_REPLAY, DEFAULT_FLUSH_MIN_DELAY } from '../../../src/constants';
 import { handleAfterSendEvent } from '../../../src/coreHandlers/handleAfterSendEvent';
 import type { ReplayContainer } from '../../../src/replay';
 import { Error } from '../../fixtures/error';
@@ -146,11 +146,18 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => {
 
     expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']);
 
-    jest.runAllTimers();
+    jest.runAllTimers()
+    await new Promise(process.nextTick);
+
+    // Flush from the error
+    expect(mockSend).toHaveBeenCalledTimes(1);
+
+    jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
     await new Promise(process.nextTick);
 
-    // Send twice, one for the error & one right after for the session conversion
+    // Flush for converting to session-based replay (startRecording call)
     expect(mockSend).toHaveBeenCalledTimes(2);
+
     // This is removed now, because it has been converted to a "session" session
     expect(Array.from(replay.getContext().errorIds)).toEqual([]);
     expect(replay.isEnabled()).toBe(true);

packages/replay/test/integration/errorSampleRate.test.ts

@@ -99,6 +99,9 @@ describe('Integration | errorSampleRate', () => {
       ]),
     });
 
+    jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
+    await new Promise(process.nextTick);
+
     // This is from when we stop recording and start a session recording
     expect(replay).toHaveLastSentReplay({
       recordingPayloadHeader: { segment_id: 1 },
@@ -116,20 +119,6 @@ describe('Integration | errorSampleRate', () => {
       ]),
     });
 
-    jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
-
-    // New checkout when we call `startRecording` again after uploading segment
-    // after an error occurs
-    expect(replay).toHaveLastSentReplay({
-      recordingData: JSON.stringify([
-        {
-          data: { isCheckout: true },
-          timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40,
-          type: 2,
-        },
-      ]),
-    });
-
     // Check that click will get captured
     domHandler({
       name: 'click',
@@ -139,14 +128,15 @@ describe('Integration | errorSampleRate', () => {
     await new Promise(process.nextTick);
 
     expect(replay).toHaveLastSentReplay({
+      recordingPayloadHeader: { segment_id: 2 },
       recordingData: JSON.stringify([
         {
           type: 5,
-          timestamp: BASE_TIMESTAMP + 10000 + 60,
+          timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 80,
           data: {
             tag: 'breadcrumb',
             payload: {
-              timestamp: (BASE_TIMESTAMP + 10000 + 60) / 1000,
+              timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 80) / 1000,
               type: 'default',
               category: 'ui.click',
               message: '<unknown>',
@@ -641,6 +631,10 @@ describe('Integration | errorSampleRate', () => {
 
     expect(replay).toHaveLastSentReplay();
 
+    // Flush from calling `stopRecording`
+    jest.runAllTimers()
+    await new Promise(process.nextTick);
+
     // Now wait after session expires - should stop recording
     mockRecord.takeFullSnapshot.mockClear();
     (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>).mockClear();
@@ -746,16 +740,16 @@ it('sends a replay after loading the session multiple times', async () => {
   captureException(new Error('testing'));
 
   await new Promise(process.nextTick);
-  jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
-  await new Promise(process.nextTick);
+  await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
 
-  expect(replay).toHaveSentReplay({
+  expect(replay).toHaveLastSentReplay({
     recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, TEST_EVENT]),
   });
 
+  await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
   // Latest checkout when we call `startRecording` again after uploading segment
   // after an error occurs (e.g. when we switch to session replay recording)
   expect(replay).toHaveLastSentReplay({
-    recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5040, type: 2 }]),
+    recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 10040, type: 2 }]),
   });
 });