From f58c0cba41f7a19a1ca7c69aadeea4873ef40ea9 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Wed, 2 Aug 2023 12:45:54 +0200
Subject: [PATCH] fix(replay): Ensure buffer sessions end after capturing an
 error

---
 packages/replay/src/session/Session.ts        |  3 +-
 .../test/integration/errorSampleRate.test.ts  | 79 +++++++++++++++++++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/packages/replay/src/session/Session.ts b/packages/replay/src/session/Session.ts
index b5ecddcbdb84..e373d50dfaa2 100644
--- a/packages/replay/src/session/Session.ts
+++ b/packages/replay/src/session/Session.ts
@@ -13,6 +13,7 @@ export function makeSession(session: Partial<Session> & { sampled: Sampled }): S
   const lastActivity = session.lastActivity || now;
   const segmentId = session.segmentId || 0;
   const sampled = session.sampled;
+  const shouldRefresh = typeof session.shouldRefresh === 'boolean' ? session.shouldRefresh : true;
 
   return {
     id,
@@ -20,6 +21,6 @@ export function makeSession(session: Partial<Session> & { sampled: Sampled }): S
     lastActivity,
     segmentId,
     sampled,
-    shouldRefresh: true,
+    shouldRefresh,
   };
 }
diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts
index f92f66078c97..3138d4075887 100644
--- a/packages/replay/test/integration/errorSampleRate.test.ts
+++ b/packages/replay/test/integration/errorSampleRate.test.ts
@@ -819,6 +819,85 @@ describe('Integration | errorSampleRate', () => {
   });
 });
 
+/**
+ * If an error happens, we switch the recordingMode to `session`, set `shouldRefresh=false` on the session,
+ * but keep `sampled=buffer`.
+ * This test should verify that if we load such a session from sessionStorage, the session is eventually correctly ended.
+ */
+it('handles buffer sessions that previously had an error', async () => {
+  // Pretend that a session is already saved before loading replay
+  WINDOW.sessionStorage.setItem(
+    REPLAY_SESSION_KEY,
+    `{"segmentId":0,"id":"fd09adfc4117477abc8de643e5a5798a","sampled":"buffer","started":${BASE_TIMESTAMP},"lastActivity":${BASE_TIMESTAMP},"shouldRefresh":false}`,
+  );
+  const { mockRecord, replay, integration } = await resetSdkMock({
+    replayOptions: {
+      stickySession: true,
+    },
+    sentryOptions: {
+      replaysOnErrorSampleRate: 1.0,
+    },
+    autoStart: false,
+  });
+  integration['_initialize']();
+
+  jest.runAllTimers();
+
+  await new Promise(process.nextTick);
+  const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
+  mockRecord._emitter(TEST_EVENT);
+
+  expect(replay).not.toHaveLastSentReplay();
+
+  // Waiting for max life should eventually stop recording
+  // We simulate a full checkout which would otherwise be done automatically
+  for (let i = 0; i < MAX_SESSION_LIFE / 60_000; i++) {
+    jest.advanceTimersByTime(60_000);
+    await new Promise(process.nextTick);
+    mockRecord.takeFullSnapshot(true);
+  }
+
+  expect(replay).not.toHaveLastSentReplay();
+  expect(replay.isEnabled()).toBe(false);
+});
+
+it('handles buffer sessions that never had an error', async () => {
+  // Pretend that a session is already saved before loading replay
+  WINDOW.sessionStorage.setItem(
+    REPLAY_SESSION_KEY,
+    `{"segmentId":0,"id":"fd09adfc4117477abc8de643e5a5798a","sampled":"buffer","started":${BASE_TIMESTAMP},"lastActivity":${BASE_TIMESTAMP}}`,
+  );
+  const { mockRecord, replay, integration } = await resetSdkMock({
+    replayOptions: {
+      stickySession: true,
+    },
+    sentryOptions: {
+      replaysOnErrorSampleRate: 1.0,
+    },
+    autoStart: false,
+  });
+  integration['_initialize']();
+
+  jest.runAllTimers();
+
+  await new Promise(process.nextTick);
+  const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
+  mockRecord._emitter(TEST_EVENT);
+
+  expect(replay).not.toHaveLastSentReplay();
+
+  // Waiting for max life should eventually stop recording
+  // We simulate a full checkout which would otherwise be done automatically
+  for (let i = 0; i < MAX_SESSION_LIFE / 60_000; i++) {
+    jest.advanceTimersByTime(60_000);
+    await new Promise(process.nextTick);
+    mockRecord.takeFullSnapshot(true);
+  }
+
+  expect(replay).not.toHaveLastSentReplay();
+  expect(replay.isEnabled()).toBe(true);
+});
+
 /**
  * This is testing a case that should only happen with error-only sessions.
  * Previously we had assumed that loading a session from session storage meant