From a7be2335913577bfbada64dccd288c9463a2fb4a Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 6 Mar 2023 19:24:24 -0500 Subject: [PATCH 1/6] feat(replay): Reduce time limit before pausing a recording 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 https://github.com/getsentry/sentry-javascript/issues/7352 --- packages/replay/src/replay.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 505d53410d1f..9a65f6527b89 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -4,7 +4,7 @@ import { captureException, getCurrentHub } from '@sentry/core'; import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types'; import { logger } from '@sentry/utils'; -import { ERROR_CHECKOUT_TIME, MAX_SESSION_LIFE, SESSION_IDLE_DURATION, WINDOW } from './constants'; +import { ERROR_CHECKOUT_TIME, SESSION_IDLE_DURATION, WINDOW } from './constants'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; import { createEventBuffer } from './eventBuffer'; import { getSession } from './session/getSession'; @@ -365,10 +365,10 @@ 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, MAX_SESSION_LIFE)) { + if (this._lastActivity && isExpired(this._lastActivity, SESSION_IDLE_DURATION)) { // Pause recording this.pause(); return; From e544b3fbd05fabaca07527f00084cd2b50128e16 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 8 Mar 2023 15:32:45 -0500 Subject: [PATCH 2/6] update tests + only pause for session replays --- packages/replay/src/replay.ts | 7 +- .../test/integration/errorSampleRate.test.ts | 47 ++- .../replay/test/integration/session.test.ts | 308 +++++------------- 3 files changed, 139 insertions(+), 223 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 1e1d680eb07b..fef6c2d13752 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -395,8 +395,11 @@ export class ReplayContainer implements ReplayContainerInterface { // 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.sessionIdle)) { - // Pause recording + if (this._lastActivity && isExpired(this._lastActivity, this.timeouts.sessionIdle) && 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; } diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 3eb2288a31fb..5bd2c3396409 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/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,8 +295,45 @@ 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 () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index d6820db61da7..8dd0a7d94b8f 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -9,6 +9,7 @@ import { WINDOW, } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; +import type { Session } from '../../src/types'; import { addEvent } from '../../src/util/addEvent'; import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import { BASE_TIMESTAMP } from '../index'; @@ -55,7 +56,9 @@ describe('Integration | session', () => { }); }); - it('creates a new session and triggers a full dom snapshot when document becomes visible after [SESSION_IDLE_DURATION]ms', () => { + // Require a "user interaction" to start a new session, visibility is not enough. This can be noisy + // (e.g. rapidly switching tabs/window focus) and leads to empty sessions. + it('does not create a new session when document becomes visible after [SESSION_IDLE_DURATION]ms', () => { Object.defineProperty(document, 'visibilityState', { configurable: true, get: function () { @@ -63,169 +66,29 @@ describe('Integration | session', () => { }, }); - const initialSession = replay.session; + const initialSession = {...replay.session} as Session; jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); document.dispatchEvent(new Event('visibilitychange')); - expect(mockRecord.takeFullSnapshot).toHaveBeenLastCalledWith(true); - - // Should have created a new session - expect(replay).not.toHaveSameSession(initialSession); - }); - - it('does not create a new session if user hides the tab and comes back within [SESSION_IDLE_DURATION] seconds', () => { - const initialSession = replay.session; - - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'hidden'; - }, - }); - document.dispatchEvent(new Event('visibilitychange')); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).toHaveSameSession(initialSession); - - // User comes back before `SESSION_IDLE_DURATION` elapses - jest.advanceTimersByTime(SESSION_IDLE_DURATION - 1); - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'visible'; - }, - }); - document.dispatchEvent(new Event('visibilitychange')); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - // Should NOT have created a new session - expect(replay).toHaveSameSession(initialSession); - }); - - it('creates a new session if user has been idle for more than 15 minutes and comes back to move their mouse', async () => { - const initialSession = replay.session; - - expect(initialSession?.id).toBeDefined(); - - // Idle for 15 minutes - const FIFTEEN_MINUTES = 15 * 60000; - jest.advanceTimersByTime(FIFTEEN_MINUTES); - - // 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, - type: 3, - }; - mockRecord._emitter(TEST_EVENT); - expect(replay).not.toHaveLastSentReplay(); - - 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. - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledWith(true); - - // Should be a new session - expect(replay).not.toHaveSameSession(initialSession); - - // Replay does not send immediately because checkout was due to expired session - expect(replay).not.toHaveLastSentReplay(); - - // Now do a click - domHandler({ - name: 'click', - }); - - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - - const newTimestamp = BASE_TIMESTAMP + FIFTEEN_MINUTES; - const breadcrumbTimestamp = newTimestamp + 20; // I don't know where this 20ms comes from - - expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, - { - type: 5, - timestamp: breadcrumbTimestamp, - data: { - tag: 'breadcrumb', - payload: { - timestamp: breadcrumbTimestamp / 1000, - type: 'default', - category: 'ui.click', - message: '', - data: {}, - }, - }, - }, - ]), - }); - }); - - it('should have a session after setup', () => { - expect(replay.session).toMatchObject({ - lastActivity: BASE_TIMESTAMP, - started: BASE_TIMESTAMP, - }); - expect(replay.session?.id).toBeDefined(); - expect(replay.session?.segmentId).toBeDefined(); - }); - - it('clears session', () => { - clearSession(replay); - expect(WINDOW.sessionStorage.getItem(REPLAY_SESSION_KEY)).toBe(null); - expect(replay.session).toBe(undefined); - }); - - it('creates a new session and triggers a full dom snapshot when document becomes visible after [SESSION_IDLE_DURATION]ms', () => { - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'visible'; - }, - }); - - const initialSession = replay.session; - - jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); - - document.dispatchEvent(new Event('visibilitychange')); - - expect(mockRecord.takeFullSnapshot).toHaveBeenLastCalledWith(true); - - // Should have created a new session - expect(replay).not.toHaveSameSession(initialSession); }); - it('creates a new session and triggers a full dom snapshot when document becomes focused after [SESSION_IDLE_DURATION]ms', () => { - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'visible'; - }, - }); - - const initialSession = replay.session; + it('does not create a new session when document becomes focused after [SESSION_IDLE_DURATION]ms', () => { + const initialSession = {...replay.session} as Session; jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); - WINDOW.dispatchEvent(new Event('focus')); + document.dispatchEvent(new Event('focus')); - expect(mockRecord.takeFullSnapshot).toHaveBeenLastCalledWith(true); - - // Should have created a new session - expect(replay).not.toHaveSameSession(initialSession); + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).toHaveSameSession(initialSession); }); it('does not create a new session if user hides the tab and comes back within [SESSION_IDLE_DURATION] seconds', () => { - const initialSession = replay.session; + const initialSession = {...replay.session} as Session; Object.defineProperty(document, 'visibilityState', { configurable: true, @@ -252,8 +115,8 @@ describe('Integration | session', () => { expect(replay).toHaveSameSession(initialSession); }); - it('creates a new session if user has been idle for 15 minutes and comes back to click their mouse', async () => { - const initialSession = replay.session; + it('creates a new session if user has been idle for more than SESSION_IDLE_DURATION and comes back to click their mouse', async () => { + const initialSession = {...replay.session} as Session; expect(initialSession?.id).toBeDefined(); expect(replay.getContext()).toEqual( @@ -268,44 +131,59 @@ describe('Integration | session', () => { value: new URL(url), }); - // Idle for 15 minutes - const FIFTEEN_MINUTES = 15 * 60000; - jest.advanceTimersByTime(FIFTEEN_MINUTES); + const ELAPSED = SESSION_IDLE_DURATION +1; + jest.advanceTimersByTime(ELAPSED); - // TBD: We are currently deciding that this event will get dropped, but - // this could/should change in the future. + // Session has become in an idle state + // + // This event will put the Replay SDK into a paused state const TEST_EVENT = { data: { name: 'lost event' }, timestamp: BASE_TIMESTAMP, type: 3, }; mockRecord._emitter(TEST_EVENT); - expect(replay).not.toHaveLastSentReplay(); - await new Promise(process.nextTick); + // performance events can still be collected while recording is stopped + // TODO: we may want to prevent `addEvent` from adding to buffer when user is inactive + replay.addUpdate(() => { + createPerformanceSpans(replay, [ + { + type: 'navigation.navigate', + name: 'foo', + start: BASE_TIMESTAMP + ELAPSED, + end: BASE_TIMESTAMP + ELAPSED + 100, + }, + ]); + return true; + }); - // 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. - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledWith(true); + await new Promise(process.nextTick); expect(replay).not.toHaveLastSentReplay(); + expect(replay.isPaused()).toBe(true); + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).toHaveSameSession(initialSession); + expect(mockRecord).toHaveBeenCalledTimes(1); - // Should be a new session - expect(replay).not.toHaveSameSession(initialSession); - - // Now do a click + // Now do a click which will create a new session and start recording again domHandler({ name: 'click', }); + // This is not called because we have to start recording + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(mockRecord).toHaveBeenCalledTimes(2); + + // Should be a new session + expect(replay).not.toHaveSameSession(initialSession); + + // Replay does not send immediately because checkout was due to expired session + expect(replay).not.toHaveLastSentReplay(); + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - const newTimestamp = BASE_TIMESTAMP + FIFTEEN_MINUTES; - const breadcrumbTimestamp = newTimestamp + 20; // I don't know where this 20ms comes from + const newTimestamp = BASE_TIMESTAMP + ELAPSED + 20; expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 0 }, @@ -313,11 +191,11 @@ describe('Integration | session', () => { { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, { type: 5, - timestamp: breadcrumbTimestamp, + timestamp: newTimestamp, data: { tag: 'breadcrumb', payload: { - timestamp: breadcrumbTimestamp / 1000, + timestamp: newTimestamp / 1000, type: 'default', category: 'ui.click', message: '', @@ -329,18 +207,35 @@ describe('Integration | session', () => { }); // `_context` should be reset when a new session is created - expect(replay.getContext()).toEqual( - expect.objectContaining({ + expect(replay.getContext()).toEqual({ + earliestEvent: null, initialUrl: 'http://dummy/', initialTimestamp: newTimestamp, - }), - ); + urls: [], + errorIds: new Set(), + traceIds: new Set(), + }); }); - it('does not record if user has been idle for more than MAX_SESSION_LIFE and only starts a new session after a user action', async () => { + it('should have a session after setup', () => { + expect(replay.session).toMatchObject({ + lastActivity: BASE_TIMESTAMP, + started: BASE_TIMESTAMP, + }); + expect(replay.session?.id).toBeDefined(); + expect(replay.session?.segmentId).toBeDefined(); + }); + + it('clears session', () => { + clearSession(replay); + expect(WINDOW.sessionStorage.getItem(REPLAY_SESSION_KEY)).toBe(null); + expect(replay.session).toBe(undefined); + }); + + it('creates a new session if current session exceeds MAX_SESSION_LIFE', async () => { jest.clearAllMocks(); - const initialSession = replay.session; + const initialSession = {...replay.session} as Session; expect(initialSession?.id).toBeDefined(); expect(replay.getContext()).toEqual( @@ -355,80 +250,55 @@ describe('Integration | session', () => { value: new URL(url), }); - // Idle for MAX_SESSION_LIFE - jest.advanceTimersByTime(MAX_SESSION_LIFE); + // Advanced past MAX_SESSION_LIFE + const ELAPSED = MAX_SESSION_LIFE + 1; + jest.advanceTimersByTime(ELAPSED); + // Update activity so as to not consider session to be idling + replay['_updateUserActivity'](); + replay['_updateSessionActivity'](); - // These events will not get flushed and will eventually be dropped because user is idle and session is expired + // This should trigger a new session const TEST_EVENT = { data: { name: 'lost event' }, - timestamp: MAX_SESSION_LIFE, + timestamp: ELAPSED, type: 3, }; mockRecord._emitter(TEST_EVENT); - // performance events can still be collected while recording is stopped - // TODO: we may want to prevent `addEvent` from adding to buffer when user is inactive - replay.addUpdate(() => { - createPerformanceSpans(replay, [ - { - type: 'navigation.navigate', - name: 'foo', - start: BASE_TIMESTAMP + MAX_SESSION_LIFE, - end: BASE_TIMESTAMP + MAX_SESSION_LIFE + 100, - }, - ]); - return true; - }); - - WINDOW.dispatchEvent(new Event('blur')); - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveSameSession(initialSession); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalled(); expect(replay).not.toHaveLastSentReplay(); - // Should be the same session because user has been idle and no events have caused a new session to be created - expect(replay).toHaveSameSession(initialSession); - // @ts-ignore private - expect(replay._stopRecording).toBeUndefined(); + expect(replay._stopRecording).toBeDefined(); // Now do a click domHandler({ name: 'click', }); - // This should still be thrown away - mockRecord._emitter(TEST_EVENT); + + const newTimestamp = BASE_TIMESTAMP + ELAPSED; const NEW_TEST_EVENT = { data: { name: 'test' }, - timestamp: BASE_TIMESTAMP + MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 20, + timestamp: newTimestamp + DEFAULT_FLUSH_MIN_DELAY + 20, type: 3, }; - mockRecord._emitter(NEW_TEST_EVENT); - // new session is created jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveSameSession(initialSession); await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - const newTimestamp = BASE_TIMESTAMP + MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 20; // I don't know where this 20ms comes from - const breadcrumbTimestamp = newTimestamp; - - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ { data: { isCheckout: true }, timestamp: newTimestamp, type: 2 }, { type: 5, - timestamp: breadcrumbTimestamp, + timestamp: newTimestamp, data: { tag: 'breadcrumb', payload: { - timestamp: breadcrumbTimestamp / 1000, + timestamp: newTimestamp / 1000, type: 'default', category: 'ui.click', message: '', From 8ac4d4b24266b307990447a1476d1272234e02ca Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 8 Mar 2023 15:43:04 -0500 Subject: [PATCH 3/6] WINDOW --- packages/replay/test/integration/session.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index 8dd0a7d94b8f..fd54437ce3d0 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -81,7 +81,7 @@ describe('Integration | session', () => { jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); - document.dispatchEvent(new Event('focus')); + WINDOW.dispatchEvent(new Event('focus')); expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).toHaveSameSession(initialSession); From c2e88504fbd6048f4a073c13ca3e40f9568aac43 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 8 Mar 2023 17:48:53 -0500 Subject: [PATCH 4/6] fix integration test --- .../integration-tests/suites/replay/sessionInactive/init.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-tests/suites/replay/sessionInactive/init.js b/packages/integration-tests/suites/replay/sessionInactive/init.js index f1b0345e71d7..a3da64ec3bae 100644 --- a/packages/integration-tests/suites/replay/sessionInactive/init.js +++ b/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 }; From ff2d561e1fac068b53d764a57160e2b46ecf85df Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 8 Mar 2023 17:57:50 -0500 Subject: [PATCH 5/6] lint --- packages/replay/src/replay.ts | 7 +++++- .../test/integration/errorSampleRate.test.ts | 9 ++++--- .../replay/test/integration/session.test.ts | 24 +++++++++---------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index fef6c2d13752..7f397855d17f 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -395,7 +395,12 @@ export class ReplayContainer implements ReplayContainerInterface { // 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.sessionIdle) && this.session?.sampled === 'session') { + 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 diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 5bd2c3396409..bc6bd269d06e 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -297,8 +297,8 @@ describe('Integration | errorSampleRate', () => { expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); domHandler({ - name: 'click' - }) + name: 'click', + }); // Remains disabled! expect(replay.isEnabled()).toBe(false); @@ -326,14 +326,13 @@ describe('Integration | errorSampleRate', () => { expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); domHandler({ - name: 'click' - }) + name: 'click', + }); // Remains disabled! expect(replay.isEnabled()).toBe(false); }); - it('has the correct timestamps with deferred root event and last replay update', async () => { const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index fd54437ce3d0..2492b5ee913b 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -66,7 +66,7 @@ describe('Integration | session', () => { }, }); - const initialSession = {...replay.session} as Session; + const initialSession = { ...replay.session } as Session; jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); @@ -77,7 +77,7 @@ describe('Integration | session', () => { }); it('does not create a new session when document becomes focused after [SESSION_IDLE_DURATION]ms', () => { - const initialSession = {...replay.session} as Session; + const initialSession = { ...replay.session } as Session; jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); @@ -88,7 +88,7 @@ describe('Integration | session', () => { }); it('does not create a new session if user hides the tab and comes back within [SESSION_IDLE_DURATION] seconds', () => { - const initialSession = {...replay.session} as Session; + const initialSession = { ...replay.session } as Session; Object.defineProperty(document, 'visibilityState', { configurable: true, @@ -116,7 +116,7 @@ describe('Integration | session', () => { }); it('creates a new session if user has been idle for more than SESSION_IDLE_DURATION and comes back to click their mouse', async () => { - const initialSession = {...replay.session} as Session; + const initialSession = { ...replay.session } as Session; expect(initialSession?.id).toBeDefined(); expect(replay.getContext()).toEqual( @@ -131,7 +131,7 @@ describe('Integration | session', () => { value: new URL(url), }); - const ELAPSED = SESSION_IDLE_DURATION +1; + const ELAPSED = SESSION_IDLE_DURATION + 1; jest.advanceTimersByTime(ELAPSED); // Session has become in an idle state @@ -208,12 +208,12 @@ describe('Integration | session', () => { // `_context` should be reset when a new session is created expect(replay.getContext()).toEqual({ - earliestEvent: null, - initialUrl: 'http://dummy/', - initialTimestamp: newTimestamp, - urls: [], - errorIds: new Set(), - traceIds: new Set(), + earliestEvent: null, + initialUrl: 'http://dummy/', + initialTimestamp: newTimestamp, + urls: [], + errorIds: new Set(), + traceIds: new Set(), }); }); @@ -235,7 +235,7 @@ describe('Integration | session', () => { it('creates a new session if current session exceeds MAX_SESSION_LIFE', async () => { jest.clearAllMocks(); - const initialSession = {...replay.session} as Session; + const initialSession = { ...replay.session } as Session; expect(initialSession?.id).toBeDefined(); expect(replay.getContext()).toEqual( From 9daaf68dcad19c044fd72e0d31ec95af7b14d4a6 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 9 Mar 2023 09:26:58 -0500 Subject: [PATCH 6/6] add another test --- .../test/integration/errorSampleRate.test.ts | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index bc6bd269d06e..1c4ef227db6c 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -417,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).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 };