feat(replay): Reduce time limit before pausing a recording (#7356)

Reduce the time limit before pausing a recording from `MAX_SESSION_LIFE` (1 hour) to `SESSION_IDLE_DURATION` (5 minutes). This reduces the amount of empty replays that are caused by a DOM mutation on a timer. An example of this is on sentry.io where there is a time component that updates every x seconds.

As a reminder, there are three events that will break the idle timeout:

* mouse click
* user input in a form field
* a navigation

Closes #7352

Author: billyvg

packages/integration-tests/suites/replay/sessionInactive/init.js

@@ -17,6 +17,6 @@ Sentry.init({
 });
 
 window.Replay._replay.timeouts = {
-  sessionIdle: 300000, // default: 5min
+  sessionIdle: 1000, // default: 5min
   maxSessionLife: 2000, // this is usually 60min, but we want to test this with shorter times
 };

packages/replay/src/replay.ts

@@ -392,11 +392,19 @@ export class ReplayContainer implements ReplayContainerInterface {
     const oldSessionId = this.getSessionId();
 
     // Prevent starting a new session if the last user activity is older than
-    // MAX_SESSION_LIFE. Otherwise non-user activity can trigger a new
+    // SESSION_IDLE_DURATION. Otherwise non-user activity can trigger a new
     // session+recording. This creates noisy replays that do not have much
     // content in them.
-    if (this._lastActivity && isExpired(this._lastActivity, this.timeouts.maxSessionLife)) {
-      // Pause recording
+    if (
+      this._lastActivity &&
+      isExpired(this._lastActivity, this.timeouts.sessionIdle) &&
+      this.session &&
+      this.session.sampled === 'session'
+    ) {
+      // Pause recording only for session-based replays. Otherwise, resuming
+      // will create a new replay and will conflict with users who only choose
+      // to record error-based replays only. (e.g. the resumed replay will not
+      // contain a reference to an error)
       this.pause();
       return;
     }

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

@@ -269,9 +269,15 @@ describe('Integration | errorSampleRate', () => {
     expect(replay).not.toHaveLastSentReplay();
   });
 
-  it('stops replay if user has been idle for more than 15 minutes and comes back to move their mouse', async () => {
+  // When the error session records as a normal session, we want to stop
+  // recording after the session ends. Otherwise, we get into a state where the
+  // new session is a session type replay (this could conflict with the session
+  // sample rate of 0.0), or an error session that has no errors. Instead we
+  // simply stop the session replay completely and wait for a new page load to
+  // resample.
+  it('stops replay if session exceeds MAX_SESSION_LIFE and does not start a new session thereafter', async () => {
     // Idle for 15 minutes
-    jest.advanceTimersByTime(15 * 60000);
+    jest.advanceTimersByTime(MAX_SESSION_LIFE + 1);
 
     const TEST_EVENT = {
       data: { name: 'lost event' },
@@ -289,6 +295,42 @@ describe('Integration | errorSampleRate', () => {
     expect(replay).not.toHaveLastSentReplay();
     expect(replay.isEnabled()).toBe(false);
     expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
+
+    domHandler({
+      name: 'click',
+    });
+
+    // Remains disabled!
+    expect(replay.isEnabled()).toBe(false);
+  });
+
+  // Should behave the same as above test
+  it('stops replay if user has been idle for more than SESSION_IDLE_DURATION and does not start a new session thereafter', async () => {
+    // Idle for 15 minutes
+    jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1);
+
+    const TEST_EVENT = {
+      data: { name: 'lost event' },
+      timestamp: BASE_TIMESTAMP,
+      type: 3,
+    };
+    mockRecord._emitter(TEST_EVENT);
+    expect(replay).not.toHaveLastSentReplay();
+
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+
+    // We stop recording after SESSION_IDLE_DURATION of inactivity in error mode
+    expect(replay).not.toHaveLastSentReplay();
+    expect(replay.isEnabled()).toBe(false);
+    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
+
+    domHandler({
+      name: 'click',
+    });
+
+    // Remains disabled!
+    expect(replay.isEnabled()).toBe(false);
   });
 
   it('has the correct timestamps with deferred root event and last replay update', async () => {
@@ -375,7 +417,46 @@ describe('Integration | errorSampleRate', () => {
     });
   });
 
-  it('stops replay when session expires', async () => {
+  it('stops replay when user goes idle', 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();
+
+    // Now wait after session expires - should stop recording
+    mockRecord.takeFullSnapshot.mockClear();
+    (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance<any>).mockClear();
+
+    // Go idle
+    jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1);
+    await new Promise(process.nextTick);
+
+    mockRecord._emitter(TEST_EVENT);
+
+    expect(replay).not.toHaveLastSentReplay();
+
+    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);
+  });
+
+  it('stops replay when session exceeds max length', async () => {
     jest.setSystemTime(BASE_TIMESTAMP);
 
     const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 };