fix(replay): Do not renew session in error mode

Author: mydea

packages/replay/src/replay.ts

@@ -264,6 +264,11 @@ export class ReplayContainer implements ReplayContainerInterface {
    * new DOM checkout.`
    */
   public resume(): void {
+    if (!this.session || !this.session.sampled) {
+      this.stop();
+      return;
+    }
+
     this._isPaused = false;
     this.startRecording();
   }
@@ -312,6 +317,13 @@ export class ReplayContainer implements ReplayContainerInterface {
       // will get rejected due to an expired session.
       this._loadSession({ expiry: SESSION_IDLE_DURATION });
 
+      // We know this is set, because it is always set in _loadSession
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      if (!this.session!.sampled) {
+        this.stop();
+        return;
+      }
+
       // Note: This will cause a new DOM checkout
       this.resume();
       return;
@@ -365,6 +377,13 @@ export class ReplayContainer implements ReplayContainerInterface {
     // This will create a new session if expired, based on expiry length
     this._loadSession({ expiry });
 
+    // We know this is set, because it is always set in _loadSession
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    if (!this.session!.sampled) {
+      this.stop();
+      return;
+    }
+
     // Session was expired if session ids do not match
     const expired = oldSessionId !== this.getSessionId();
 

packages/replay/src/session/getSession.ts

@@ -4,6 +4,7 @@ import type { Session, SessionOptions } from '../types';
 import { isSessionExpired } from '../util/isSessionExpired';
 import { createSession } from './createSession';
 import { fetchSession } from './fetchSession';
+import { makeSession } from './Session';
 
 interface GetSessionParams extends SessionOptions {
   /**
@@ -38,6 +39,10 @@ export function getSession({
 
     if (!isExpired) {
       return { type: 'saved', session };
+    } else if (session.sampled === 'error') {
+      // Error samples should not be re-created when expired, but instead we stop when the replay is done
+      const discardedSession = makeSession({ sampled: false });
+      return { type: 'new', session: discardedSession };
     } else {
       __DEBUG_BUILD__ && logger.log('[Replay] Session has expired');
     }

packages/replay/test/integration/errorSampleRate.test.ts

@@ -1,8 +1,9 @@
-import { captureException } from '@sentry/core';
+import { captureException, getCurrentHub } from '@sentry/core';
 
 import {
   DEFAULT_FLUSH_MIN_DELAY,
   ERROR_CHECKOUT_TIME,
+  MAX_SESSION_LIFE,
   REPLAY_SESSION_KEY,
   VISIBILITY_CHANGE_TIMEOUT,
   WINDOW,
@@ -264,12 +265,10 @@ describe('Integration | errorSampleRate', () => {
     expect(replay).not.toHaveLastSentReplay();
   });
 
-  it('does not upload if user has been idle for more than 15 minutes and comes back to move their mouse', async () => {
+  it('stops replay if user has been idle for more than 15 minutes and comes back to move their mouse', async () => {
     // Idle for 15 minutes
     jest.advanceTimersByTime(15 * 60000);
 
-    // TBD: We are currently deciding that this event will get dropped, but
-    // this could/should change in the future.
     const TEST_EVENT = {
       data: { name: 'lost event' },
       timestamp: BASE_TIMESTAMP,
@@ -281,15 +280,11 @@ describe('Integration | errorSampleRate', () => {
     jest.runAllTimers();
     await new Promise(process.nextTick);
 
-    // Instead of recording the above event, a full snapshot will occur.
-    //
-    // TODO: We could potentially figure out a way to save the last session,
-    // and produce a checkout based on a previous checkout + updates, and then
-    // replay the event on top. Or maybe replay the event on top of a refresh
-    // snapshot.
+    // We stop recording after 15 minutes of inactivity in error mode
 
     expect(replay).not.toHaveLastSentReplay();
-    expect(mockRecord.takeFullSnapshot).toHaveBeenCalledWith(true);
+    expect(replay.isEnabled()).toBe(false);
+    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
   });
 
   it('has the correct timestamps with deferred root event and last replay update', async () => {
@@ -375,6 +370,52 @@ describe('Integration | errorSampleRate', () => {
       ]),
     });
   });
+
+  it('stops replay when session expires', async () => {
+    jest.setSystemTime(BASE_TIMESTAMP);
+
+    const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };
+    mockRecord._emitter(TEST_EVENT);
+
+    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
+    expect(replay).not.toHaveLastSentReplay();
+
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+
+    captureException(new Error('testing'));
+
+    jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
+    await new Promise(process.nextTick);
+
+    expect(replay).toHaveLastSentReplay();
+
+    // Wait a bit, shortly before session expires
+    jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000);
+    await new Promise(process.nextTick);
+
+    mockRecord._emitter(TEST_EVENT);
+    replay.triggerUserActivity();
+
+    expect(replay).toHaveLastSentReplay();
+
+    // Now wait after session expires - should stop recording
+    mockRecord.takeFullSnapshot.mockClear();
+    (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>).mockClear();
+
+    jest.advanceTimersByTime(10_000);
+    await new Promise(process.nextTick);
+
+    mockRecord._emitter(TEST_EVENT);
+    replay.triggerUserActivity();
+
+    jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
+    await new Promise(process.nextTick);
+
+    expect(replay).not.toHaveLastSentReplay();
+    expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0);
+    expect(replay.isEnabled()).toBe(false);
+  });
 });
 
 /**