Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
14 changes: 11 additions & 3 deletions packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
87 changes: 84 additions & 3 deletions packages/replay/test/integration/errorSampleRate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 };
Expand Down
Loading