fix(replay): Fix re-sampled sessions after a click

dev-packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts

@@ -1,4 +1,5 @@
 import { expect } from '@playwright/test';
+import type { replayIntegration as actualReplayIntegration } from '@sentry-internal/replay';
 import { sentryTest } from '../../../utils/fixtures';
 import { getExpectedReplayEvent } from '../../../utils/replayEventTemplates';
 import {
@@ -13,55 +14,122 @@ import {
 // Session should expire after 2s - keep in sync with init.js
 const SESSION_TIMEOUT = 2000;
 
-sentryTest('handles an expired session', async ({ browserName, forceFlushReplay, getLocalTestUrl, page }) => {
-  if (shouldSkipReplayTest() || browserName !== 'chromium') {
-    sentryTest.skip();
-  }
+sentryTest(
+  'handles an expired session that re-samples to session',
+  async ({ browserName, forceFlushReplay, getLocalTestUrl, page }) => {
+    if (shouldSkipReplayTest() || browserName !== 'chromium') {
+      sentryTest.skip();
+    }
 
-  const reqPromise0 = waitForReplayRequest(page, 0);
-  const reqPromise1 = waitForReplayRequest(page, 1);
+    const reqPromise0 = waitForReplayRequest(page, 0);
+    const reqPromise1 = waitForReplayRequest(page, 1);
 
-  const url = await getLocalTestUrl({ testDir: __dirname });
+    const url = await getLocalTestUrl({ testDir: __dirname });
 
-  await page.goto(url);
-  const req0 = await reqPromise0;
+    await page.goto(url);
+    const req0 = await reqPromise0;
 
-  const replayEvent0 = getReplayEvent(req0);
-  expect(replayEvent0).toEqual(getExpectedReplayEvent({}));
+    const replayEvent0 = getReplayEvent(req0);
+    expect(replayEvent0).toEqual(getExpectedReplayEvent({}));
 
-  const fullSnapshots0 = getFullRecordingSnapshots(req0);
-  expect(fullSnapshots0.length).toEqual(1);
-  const stringifiedSnapshot = normalize(fullSnapshots0[0]);
-  expect(stringifiedSnapshot).toMatchSnapshot('snapshot-0.json');
+    const fullSnapshots0 = getFullRecordingSnapshots(req0);
+    expect(fullSnapshots0.length).toEqual(1);
+    const stringifiedSnapshot = normalize(fullSnapshots0[0]);
+    expect(stringifiedSnapshot).toMatchSnapshot('snapshot-0.json');
 
-  // We wait for another segment 0
-  const reqPromise2 = waitForReplayRequest(page, 0);
+    // We wait for another segment 0
+    const reqPromise2 = waitForReplayRequest(page, 0);
 
-  await page.locator('#button1').click();
-  await forceFlushReplay();
-  const req1 = await reqPromise1;
+    await page.locator('#button1').click();
+    await forceFlushReplay();
+    const req1 = await reqPromise1;
 
-  const replayEvent1 = getReplayEvent(req1);
-  expect(replayEvent1).toEqual(getExpectedReplayEvent({ segment_id: 1, urls: [] }));
+    const replayEvent1 = getReplayEvent(req1);
+    expect(replayEvent1).toEqual(getExpectedReplayEvent({ segment_id: 1, urls: [] }));
 
-  const replay = await getReplaySnapshot(page);
-  const oldSessionId = replay.session?.id;
+    const replay = await getReplaySnapshot(page);
+    const oldSessionId = replay.session?.id;
 
-  await new Promise(resolve => setTimeout(resolve, SESSION_TIMEOUT));
+    await new Promise(resolve => setTimeout(resolve, SESSION_TIMEOUT));
 
-  await page.locator('#button2').click();
-  await forceFlushReplay();
-  const req2 = await reqPromise2;
+    await page.locator('#button2').click();
+    await forceFlushReplay();
+    const req2 = await reqPromise2;
 
-  const replay2 = await getReplaySnapshot(page);
+    const replay2 = await getReplaySnapshot(page);
 
-  expect(replay2.session?.id).not.toEqual(oldSessionId);
+    expect(replay2.session?.id).not.toEqual(oldSessionId);
 
-  const replayEvent2 = getReplayEvent(req2);
-  expect(replayEvent2).toEqual(getExpectedReplayEvent({}));
+    const replayEvent2 = getReplayEvent(req2);
+    expect(replayEvent2).toEqual(getExpectedReplayEvent({}));
 
-  const fullSnapshots2 = getFullRecordingSnapshots(req2);
-  expect(fullSnapshots2.length).toEqual(1);
-  const stringifiedSnapshot2 = normalize(fullSnapshots2[0]);
-  expect(stringifiedSnapshot2).toMatchSnapshot('snapshot-2.json');
-});
+    const fullSnapshots2 = getFullRecordingSnapshots(req2);
+    expect(fullSnapshots2.length).toEqual(1);
+    const stringifiedSnapshot2 = normalize(fullSnapshots2[0]);
+    expect(stringifiedSnapshot2).toMatchSnapshot('snapshot-2.json');
+  },
+);
+
+sentryTest(
+  'handles an expired session that re-samples to buffer',
+  async ({ browserName, forceFlushReplay, getLocalTestUrl, page }) => {
+    if (shouldSkipReplayTest() || browserName !== 'chromium') {
+      sentryTest.skip();
+    }
+
+    const reqPromise0 = waitForReplayRequest(page, 0);
+    const reqPromise1 = waitForReplayRequest(page, 1);
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+
+    await page.goto(url);
+    const req0 = await reqPromise0;
+
+    const replayEvent0 = getReplayEvent(req0);
+    expect(replayEvent0).toEqual(getExpectedReplayEvent({}));
+
+    const fullSnapshots0 = getFullRecordingSnapshots(req0);
+    expect(fullSnapshots0.length).toEqual(1);
+    const stringifiedSnapshot = normalize(fullSnapshots0[0]);
+    expect(stringifiedSnapshot).toMatchSnapshot('snapshot-0.json');
+
+    await page.locator('#button1').click();
+    await forceFlushReplay();
+    const req1 = await reqPromise1;
+
+    const replayEvent1 = getReplayEvent(req1);
+    expect(replayEvent1).toEqual(getExpectedReplayEvent({ segment_id: 1, urls: [] }));
+
+    const replay = await getReplaySnapshot(page);
+    const oldSessionId = replay.session?.id;
+
+    await new Promise(resolve => setTimeout(resolve, SESSION_TIMEOUT));
+    await page.evaluate(() => {
+      const replayIntegration = (window as unknown as Window & { Replay: ReturnType<typeof actualReplayIntegration> })
+        .Replay;
+      replayIntegration['_replay'].getOptions().errorSampleRate = 1.0;
+      replayIntegration['_replay'].getOptions().sessionSampleRate = 0.0;
+    });
+
+    let wasReplayFlushed = false;
+    page.on('request', request => {
+      if (request.url().includes('/api/1337/envelope/')) {
+        wasReplayFlushed = true;
+      }
+    });
+    await page.locator('#button2').click();
+    await forceFlushReplay();
+
+    // This timeout is not ideal, but not sure of a better way to ensure replay is not flushed
+    await new Promise(resolve => setTimeout(resolve, SESSION_TIMEOUT));
+
+    expect(wasReplayFlushed).toBe(false);
+
+    const currentSessionId = await page.evaluate(() => {
+      // @ts-expect-error - Replay is not typed
+      return window.Replay._replay.session?.id;
+    });
+
+    expect(currentSessionId).not.toEqual(oldSessionId);
+  },
+);

packages/replay-internal/src/replay.ts

@@ -392,6 +392,7 @@ export class ReplayContainer implements ReplayContainerInterface {
     );
 
     this.session = session;
+    this.recordingMode = 'session';
 
     this._initializeRecording();
   }
@@ -503,6 +504,10 @@ export class ReplayContainer implements ReplayContainerInterface {
     // enter into an infinite loop when `stop()` is called while flushing.
     this._isEnabled = false;
 
+    // Make sure to reset `recordingMode` to `buffer` to avoid any additional
+    // breadcrumbs to trigger a flush (e.g. in `addUpdate()`)
+    this.recordingMode = 'buffer';
+
     try {
       DEBUG_BUILD && debug.log(`Stopping Replay${reason ? ` triggered by ${reason}` : ''}`);
 
@@ -623,7 +628,7 @@ export class ReplayContainer implements ReplayContainerInterface {
 
     // If this option is turned on then we will only want to call `flush`
     // explicitly
-    if (this.recordingMode === 'buffer') {
+    if (this.recordingMode === 'buffer' || !this._isEnabled) {
       return;
     }
 

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

@@ -207,14 +207,39 @@ describe('Integration | session', () => {
     await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_MIN_DELAY);
     await new Promise(process.nextTick);
 
-    const newTimestamp = BASE_TIMESTAMP + ELAPSED;
+    // The click actually does not trigger a flush because it never gets added to event buffer because
+    // the session is expired. We stop recording and re-sample the session expires.
+    expect(replay).not.toHaveLastSentReplay();
+
+    // This click will trigger a flush now that the session is active
+    // (sessionSampleRate=1 when resampling)
+    domHandler({
+      name: 'click',
+      event: new Event('click'),
+    });
+    await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_MIN_DELAY);
+    await new Promise(process.nextTick);
+    const newTimestamp = BASE_TIMESTAMP + ELAPSED + DEFAULT_FLUSH_MIN_DELAY;
 
     expect(replay).toHaveLastSentReplay({
       recordingPayloadHeader: { segment_id: 0 },
       recordingData: JSON.stringify([
-        { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 },
+        { data: { isCheckout: true }, timestamp: newTimestamp - DEFAULT_FLUSH_MIN_DELAY, type: 2 },
         optionsEvent,
-        // the click is lost, but that's OK
+        {
+          type: 5,
+          timestamp: newTimestamp,
+          data: {
+            tag: 'breadcrumb',
+            payload: {
+              timestamp: newTimestamp / 1000,
+              type: 'default',
+              category: 'ui.click',
+              message: '<unknown>',
+              data: {},
+            },
+          },
+        },
       ]),
     });
 
@@ -224,7 +249,7 @@ describe('Integration | session', () => {
     // `_context` should be reset when a new session is created
     expect(replay.getContext()).toEqual({
       initialUrl: 'http://dummy/',
-      initialTimestamp: newTimestamp,
+      initialTimestamp: newTimestamp - DEFAULT_FLUSH_MIN_DELAY,
       urls: [],
       errorIds: new Set(),
       traceIds: new Set(),