fix(replay): Use session.started for min/max duration check (#8617)

We've been using `context.initialTimestamp` to check for min/max session duration, but actually this is always updated when a tab is refreshed, for example. Instead, we now use `session.started`, which should also always be updated for a buffer session. --------- Co-authored-by: Billy Vong <[email protected]>
getsentry · Jul 25, 2023 · 893330a · 893330a
1 parent d4fce08
commit 893330a
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 4 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/init.js b/packages/browser-integration-tests/suites/replay/maxReplayDuration/init.js
@@ -0,0 +1,23 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+window.Replay = new Sentry.Replay({
+  flushMinDelay: 200,
+  flushMaxDelay: 200,
+  minReplayDuration: 0,
+});
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  sampleRate: 0,
+  replaysSessionSampleRate: 1.0,
+  replaysOnErrorSampleRate: 0.0,
+
+  integrations: [window.Replay],
+});
+
+window.Replay._replay.timeouts = {
+  sessionIdlePause: 1000, // this is usually 5min, but we want to test this with shorter times
+  sessionIdleExpire: 2000, // this is usually 15min, but we want to test this with shorter times
+  maxSessionLife: 2000, // default: 60min
+};
diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/template.html b/packages/browser-integration-tests/suites/replay/maxReplayDuration/template.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button onclick="console.log('Test log 1')" id="button1">Click me</button>
+    <button onclick="console.log('Test log 2')" id="button2">Click me</button>
+  </body>
+</html>
diff --git a/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts b/packages/browser-integration-tests/suites/replay/maxReplayDuration/test.ts
@@ -0,0 +1,60 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { getExpectedReplayEvent } from '../../../utils/replayEventTemplates';
+import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';
+
+const SESSION_MAX_AGE = 2000;
+
+sentryTest('keeps track of max duration across reloads', async ({ getLocalTestPath, page }) => {
+  if (shouldSkipReplayTest()) {
+    sentryTest.skip();
+  }
+
+  const reqPromise0 = waitForReplayRequest(page, 0);
+  const reqPromise1 = waitForReplayRequest(page, 1);
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  await page.goto(url);
+
+  await new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2));
+
+  await page.reload();
+  await page.click('#button1');
+
+  // After the second reload, we should have a new session (because we exceeded max age)
+  const reqPromise3 = waitForReplayRequest(page, 0);
+
+  await new Promise(resolve => setTimeout(resolve, SESSION_MAX_AGE / 2 + 100));
+
+  void page.click('#button1');
+  await page.evaluate(`Object.defineProperty(document, 'visibilityState', {
+    configurable: true,
+    get: function () {
+      return 'hidden';
+    },
+  });
+  document.dispatchEvent(new Event('visibilitychange'));`);
+
+  const replayEvent0 = getReplayEvent(await reqPromise0);
+  expect(replayEvent0).toEqual(getExpectedReplayEvent({}));
+
+  const replayEvent1 = getReplayEvent(await reqPromise1);
+  expect(replayEvent1).toEqual(
+    getExpectedReplayEvent({
+      segment_id: 1,
+    }),
+  );
+
+  const replayEvent3 = getReplayEvent(await reqPromise3);
+  expect(replayEvent3).toEqual(getExpectedReplayEvent({}));
+});
diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts
@@ -426,6 +426,10 @@ export class ReplayContainer implements ReplayContainerInterface {
 
     const activityTime = Date.now();
 
+    // eslint-disable-next-line no-console
+    const log = this.getOptions()._experiments.traceInternals ? console.info : logger.info;
+    __DEBUG_BUILD__ && log(`[Replay] Converting buffer to session, starting at ${activityTime}`);
+
     // Allow flush to complete before resuming as a session recording, otherwise
     // the checkout from `startRecording` may be included in the payload.
     // Prefer to keep the error replay as a separate (and smaller) segment
@@ -981,9 +985,6 @@ export class ReplayContainer implements ReplayContainerInterface {
 
     const earliestEvent = eventBuffer.getEarliestTimestamp();
     if (earliestEvent && earliestEvent < this._context.initialTimestamp) {
-      // eslint-disable-next-line no-console
-      const log = this.getOptions()._experiments.traceInternals ? console.info : logger.info;
-      __DEBUG_BUILD__ && log(`[Replay] Updating initial timestamp to ${earliestEvent}`);
       this._context.initialTimestamp = earliestEvent;
     }
   }
@@ -1103,7 +1104,7 @@ export class ReplayContainer implements ReplayContainerInterface {
       return;
     }
 
-    const start = this._context.initialTimestamp;
+    const start = this.session.started;
     const now = Date.now();
     const duration = now - start;
 

diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts
@@ -72,6 +72,11 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
       if (replay.recordingMode === 'buffer' && replay.session && replay.eventBuffer) {
         const earliestEvent = replay.eventBuffer.getEarliestTimestamp();
         if (earliestEvent) {
+          // eslint-disable-next-line no-console
+          const log = replay.getOptions()._experiments.traceInternals ? console.info : logger.info;
+          __DEBUG_BUILD__ &&
+            log(`[Replay] Updating session start time to earliest event in buffer at ${earliestEvent}`);
+
           replay.session.started = earliestEvent;
 
           if (replay.getOptions().stickySession) {