feat: Stop recording when user is idle by billyvg · Pull Request #261 · getsentry/sentry-replay

billyvg · 2022-10-26T22:02:13Z

If a user has not performed any user activity for MAX_SESSION_LIFE (30 minutes currently), stop dom recording until a user performs an action (currently: mouse click, navigation, tab focus).

Previously, dom recording would trigger new sessions indefinitely.

Fixes #204

If a user has not performed any user activity for MAX_SESSION_LIFE (30 minutes currently), stop dom recording until a user performs an action (currently: mouse click, navigation, tab focus). Previously, dom recording would trigger new sessions indefinitely.

github-actions · 2022-10-26T22:03:27Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
dist/index.js	33.65 KB (+2.03% 🔺)	674 ms (+2.03% 🔺)	248 ms (+245.2% 🔺)	921 ms

billyvg · 2022-10-26T22:02:50Z

src/coreHandlers/handleDom.ts

 import { record } from 'rrweb';

-import createBreadcrumb from '../util/createBreadcrumb';
+import { createBreadcrumb } from '../util/createBreadcrumb';


For consistency

billyvg · 2022-10-26T22:21:02Z

src/index.ts

+  /**
+   * We want to batch uploads of replay events. Save events only if
+   * `<flushMinDelay>` milliseconds have elapsed since the last event
+   * *OR* if `<flushMaxDelay>` milliseconds have elapsed.
+   *
+   * Accepts a callback to perform side-effects and returns true to stop batch
+   * processing and hand back control to caller.
+   */
+  addUpdate(cb?: AddUpdateCallback) {
+    // We need to always run `cb` (e.g. in the case of captureOnlyOnError == true)
+    const cbResult = cb?.();
+
+    // If this option is turned on then we will only want to call `flush`
+    // explicitly
+    if (this.options.captureOnlyOnError) {
+      return;
+    }
+
+    // If callback is true, we do not want to continue with flushing -- the
+    // caller will need to handle it.
+    if (cbResult === true) {
+      return;
+    }
+
+    // addUpdate is called quite frequently - use debouncedFlush so that it
+    // respects the flush delays and does not flush immediately
+    this.debouncedFlush();
+  }
+


Unchanged, just moved.

billyvg · 2022-10-26T22:22:16Z

src/index.ts

-      this.loadSession({ expiry: VISIBILITY_CHANGE_TIMEOUT });
-      this.triggerFullSnapshot();


This is replaced by checkAndHandleExpiredSession call.

billyvg · 2022-10-26T22:28:55Z

src/index.ts

+    if (updateUserActivity) {
+      this.lastActivity = new Date().getTime();
+    }
+
+    // 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+recording. This creates noisy replays that do not have much
+    // content in them.
+    if (this.lastActivity && isExpired(this.lastActivity, MAX_SESSION_LIFE)) {
+      // We should stop recording emitter if it exists
+      if (this.stopRecording) {
+        this.stopRecording();
+        this.stopRecording = undefined;
+      }
+
+      return;
+    }
+
+    // --- There is recent user activity --- //
+
+    // Ensure that recording is resumed
+    if (!this.stopRecording) {
+      this.startRecording();
+    }


Basically the bulk of the change.

billyvg · 2022-10-26T22:30:00Z

src/index.ts

      }

      if (result.category === 'ui.click') {
+        this.checkAndHandleExpiredSession({ updateUserActivity: true });


I think this actually fixes a bug where we update the session last activity without checking if it's expired yet. This means we could extend beyond the 5 minute expiry.

billyvg · 2022-10-28T20:39:08Z

src/index.ts

+
+    // Ensure that recording is resumed
+    if (!this.stopRecording) {
+      this.startRecording();


This actually caused an infinite loop since it would trigger a fullsnapshot, and eventually back into this function. I've added an intermediate "paused" state to handle this.

billyvg · 2022-10-28T20:40:01Z

src/index.ts

-        this.checkAndHandleExpiredSession({ updateUserActivity: true });
-        this.updateLastActivity();


Refactored this a bit so we don't end up in a cycle

billyvg · 2022-10-28T20:41:20Z

src/index.ts

+    if (this.isPaused) {
+      // Do not add to event buffer when recording is paused
+      return;
+    }
+


This is to handle a separate bug I found. The core SDK event listeners were adding to the eventBuffer which was not being cleared on new DOM checkouts, so it was possible to have stale events in here. Now when a session goes idle, we do not add to event buffer at all.

billyvg · 2022-10-28T20:42:14Z

Found a bug with the session idle pause/resume logic so had to make some changes.

ryan953 · 2022-10-28T22:03:32Z

src/index.ts

+      // Create a new session, otherwise when the user action is flushed, it will get rejected due to an expired session.
+      this.loadSession({ expiry: SESSION_IDLE_DURATION });
+
+      // Note: This will cause a new checkout


guaranteed to start a new checkout? or will it do so only if needed

Yeah guaranteed.

If a user has not performed any user activity for MAX_SESSION_LIFE (30 minutes currently), stop dom recording until a user performs an action (currently: mouse click, navigation, tab focus). Previously, dom recording would trigger new sessions indefinitely. Fixes getsentry/sentry-replay#204

refactor

fb1274c

billyvg commented Oct 26, 2022

View reviewed changes

billyvg marked this pull request as ready for review October 26, 2022 22:30

billyvg requested a review from a team October 26, 2022 22:30

ryan953 approved these changes Oct 27, 2022

View reviewed changes

add paused state

9fb4f1b

billyvg commented Oct 28, 2022

View reviewed changes

billyvg requested a review from a team October 28, 2022 20:41

ryan953 reviewed Oct 28, 2022

View reviewed changes

ryan953 approved these changes Oct 28, 2022

View reviewed changes

update comments

ef76aa2

billyvg merged commit dbcdb19 into main Oct 31, 2022

billyvg deleted the feat/stop-recording-if-user-is-idle branch October 31, 2022 16:44

billyvg mentioned this pull request Nov 15, 2022

Stop plugin after a "max" idle time has been reached #213

Closed

billyvg mentioned this pull request Mar 6, 2023

Fix "empty" replays due to idle sessions + non-user initiated events getsentry/sentry-javascript#7352

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Stop recording when user is idle#261

feat: Stop recording when user is idle#261
billyvg merged 4 commits intomainfrom
feat/stop-recording-if-user-is-idle

billyvg commented Oct 26, 2022 •

edited

Loading

Uh oh!

github-actions bot commented Oct 26, 2022 •

edited

Loading

Uh oh!

billyvg Oct 26, 2022

Uh oh!

billyvg Oct 26, 2022

Uh oh!

billyvg Oct 26, 2022

Uh oh!

billyvg Oct 26, 2022

Uh oh!

billyvg Oct 26, 2022

Uh oh!

billyvg Oct 28, 2022

Uh oh!

billyvg Oct 28, 2022

Uh oh!

billyvg Oct 28, 2022

Uh oh!

billyvg commented Oct 28, 2022

Uh oh!

ryan953 Oct 28, 2022

Uh oh!

billyvg Oct 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		this.loadSession({ expiry: VISIBILITY_CHANGE_TIMEOUT });
		this.triggerFullSnapshot();

		this.checkAndHandleExpiredSession({ updateUserActivity: true });
		this.updateLastActivity();

Uh oh!

Conversation

billyvg commented Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

billyvg commented Oct 28, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

billyvg commented Oct 26, 2022 •

edited

Loading

github-actions bot commented Oct 26, 2022 •

edited

Loading