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

This fixes a bug where an expired session-based replay will always force a new session-based replay after a click, regardless of the actual recording mode.

What is happening is:

- a session-based replay is left idle
- user comes back and clicks on the page and triggers the click listener
- `addBreadcrumbEvent()` is called which then calls `triggerUserActivity()` because it is a click
- next, `_checkSession()` and `_refreshSession()` are called and this is where the problem starts

Inside of `_refreshSession` we stop the current replay (because the session is expired), however `stop()` is async and is `await`-ed before we re-sample. So the current replay state while `stop()` is finishing has:

- `recordingMode` = `session` (initial value)
- `isEnabled` = false

Another however, `addBreadcrumbEvent` (and everything called until `_refreshSession`) are not async and does wait for resampling (`initializeSampling()`) to occur. This means that the click breadcrumb ends up causing a flush and always starting a new replay recording.

Author: billyvg

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

@@ -13,7 +13,7 @@ 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 }) => {
+sentryTest('handles an expired session that re-samples to session', async ({ browserName, forceFlushReplay, getLocalTestUrl, page }) => {
   if (shouldSkipReplayTest() || browserName !== 'chromium') {
     sentryTest.skip();
   }
@@ -65,3 +65,64 @@ sentryTest('handles an expired session', async ({ browserName, forceFlushReplay,
   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(() => {
+    // @ts-expect-error - Replay is not typed
+    window.Replay._replay._options.errorSampleRate = 1;
+    // @ts-expect-error - Replay is not typed
+    window.Replay._replay._options.sessionSampleRate = 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

@@ -503,6 +503,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 +627,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;
     }