fix(replay): Ensure buffer->session switch is reliable (#8712)

This does two things: 1. Ensure we switch the `recordingMode` only once, to avoid running this process twice if e.g. multiple errors happen 2. Do not update `session.started` property when switching buffer->session. We just keep started at whatever was the first event sent. --------- Co-authored-by: Billy Vong <[email protected]>
getsentry · Aug 3, 2023 · e586397 · e586397
1 parent 2748691
commit e586397
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 5 deletions.
diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts
@@ -462,10 +462,12 @@ export class ReplayContainer implements ReplayContainerInterface {
       return;
     }
 
-    // Re-start recording, but in "session" recording mode
+    // To avoid race conditions where this is called multiple times, we check here again that we are still buffering
+    if ((this.recordingMode as ReplayRecordingMode) === 'session') {
+      return;
+    }
 
-    // Reset all "capture on error" configuration before
-    // starting a new recording
+    // Re-start recording in session-mode
     this.recordingMode = 'session';
 
     // Once this session ends, we do not want to refresh it
@@ -482,7 +484,6 @@ export class ReplayContainer implements ReplayContainerInterface {
       // (length of buffer), which we are ok with.
       this._updateUserActivity(activityTime);
       this._updateSessionActivity(activityTime);
-      this.session.started = activityTime;
       this._maybeSaveSession();
     }
 

diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts
@@ -226,6 +226,71 @@ describe('Integration | errorSampleRate', () => {
     });
   });
 
+  it('handles multiple simultaneous flushes', async () => {
+    const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
+    mockRecord._emitter(TEST_EVENT);
+    const optionsEvent = createOptionsEvent(replay);
+
+    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
+    expect(replay).not.toHaveLastSentReplay();
+
+    // Does not capture on mouse click
+    domHandler({
+      name: 'click',
+    });
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+    expect(replay).not.toHaveLastSentReplay();
+
+    replay.sendBufferedReplayOrFlush({ continueRecording: true });
+    replay.sendBufferedReplayOrFlush({ continueRecording: true });
+
+    await waitForBufferFlush();
+
+    expect(replay).toHaveSentReplay({
+      recordingPayloadHeader: { segment_id: 0 },
+      replayEventPayload: expect.objectContaining({
+        replay_type: 'buffer',
+      }),
+      recordingData: JSON.stringify([
+        { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 },
+        optionsEvent,
+        TEST_EVENT,
+        {
+          type: 5,
+          timestamp: BASE_TIMESTAMP,
+          data: {
+            tag: 'breadcrumb',
+            payload: {
+              timestamp: BASE_TIMESTAMP / 1000,
+              type: 'default',
+              category: 'ui.click',
+              message: '<unknown>',
+              data: {},
+            },
+          },
+        },
+      ]),
+    });
+
+    jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
+    // Check that click will not get captured
+    domHandler({
+      name: 'click',
+    });
+
+    await waitForFlush();
+
+    // This is still the last replay sent since we passed `continueRecording:
+    // false`.
+    expect(replay).toHaveLastSentReplay({
+      recordingPayloadHeader: { segment_id: 1 },
+      replayEventPayload: expect.objectContaining({
+        replay_type: 'buffer',
+      }),
+    });
+  });
+
   // This tests a regression where we were calling flush indiscriminantly in `stop()`
   it('does not upload a replay event if error is not sampled', async () => {
     // We are trying to replicate the case where error rate is 0 and session
@@ -620,7 +685,8 @@ describe('Integration | errorSampleRate', () => {
 
     await waitForBufferFlush();
 
-    expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + TICK + TICK);
+    // This is still the timestamp from the full snapshot we took earlier
+    expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + TICK);
 
     // Does not capture mouse click
     expect(replay).toHaveSentReplay({