From f69efd9abc76bbdbc2bdab102c95126286a9b88e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 1 Mar 2023 09:25:06 +0100 Subject: [PATCH] ref(replay): Use `SESSION_IDLE_DURATION` instead of `VISIBILITY_CHANGE_TIMEOUT` --- packages/replay/src/constants.ts | 3 -- packages/replay/src/replay.ts | 22 ++++++--------- .../test/integration/errorSampleRate.test.ts | 10 +++---- .../replay/test/integration/events.test.ts | 4 +-- .../replay/test/integration/flush.test.ts | 4 +-- .../test/integration/rateLimiting.test.ts | 6 ++-- .../test/integration/sendReplayEvent.test.ts | 6 ++-- .../replay/test/integration/session.test.ts | 28 +++++++++---------- packages/replay/test/integration/stop.test.ts | 4 +-- .../replay/test/utils/setupReplayContainer.ts | 3 +- 10 files changed, 40 insertions(+), 50 deletions(-) diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index 85c482133ba9..c3550cb988c3 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -14,9 +14,6 @@ export const UNABLE_TO_SEND_REPLAY = 'Unable to send Replay'; // The idle limit for a session export const SESSION_IDLE_DURATION = 300_000; // 5 minutes in ms -// Grace period to keep a session when a user changes tabs or hides window -export const VISIBILITY_CHANGE_TIMEOUT = SESSION_IDLE_DURATION; - // The maximum length of a session export const MAX_SESSION_LIFE = 3_600_000; // 60 minutes diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 413286af5e82..505d53410d1f 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -4,13 +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, - VISIBILITY_CHANGE_TIMEOUT, - WINDOW, -} from './constants'; +import { ERROR_CHECKOUT_TIME, MAX_SESSION_LIFE, SESSION_IDLE_DURATION, WINDOW } from './constants'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; import { createEventBuffer } from './eventBuffer'; import { getSession } from './session/getSession'; @@ -367,7 +361,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Returns true if session is not expired, false otherwise. * @hidden */ - public checkAndHandleExpiredSession(expiry?: number): boolean | void { + public checkAndHandleExpiredSession(): boolean | void { const oldSessionId = this.getSessionId(); // Prevent starting a new session if the last user activity is older than @@ -382,7 +376,7 @@ export class ReplayContainer implements ReplayContainerInterface { // --- There is recent user activity --- // // This will create a new session if expired, based on expiry length - if (!this._loadAndCheckSession(expiry)) { + if (!this._loadAndCheckSession()) { return; } @@ -412,9 +406,9 @@ export class ReplayContainer implements ReplayContainerInterface { * Loads (or refreshes) the current session. * Returns false if session is not recorded. */ - private _loadAndCheckSession(expiry = SESSION_IDLE_DURATION): boolean { + private _loadAndCheckSession(): boolean { const { type, session } = getSession({ - expiry, + expiry: SESSION_IDLE_DURATION, stickySession: Boolean(this._options.stickySession), currentSession: this.session, sessionSampleRate: this._options.sessionSampleRate, @@ -626,7 +620,7 @@ export class ReplayContainer implements ReplayContainerInterface { return; } - const expired = isSessionExpired(this.session, VISIBILITY_CHANGE_TIMEOUT); + const expired = isSessionExpired(this.session, SESSION_IDLE_DURATION); if (breadcrumb && !expired) { this._createCustomBreadcrumb(breadcrumb); @@ -646,10 +640,10 @@ export class ReplayContainer implements ReplayContainerInterface { return; } - const isSessionActive = this.checkAndHandleExpiredSession(VISIBILITY_CHANGE_TIMEOUT); + const isSessionActive = this.checkAndHandleExpiredSession(); if (!isSessionActive) { - // If the user has come back to the page within VISIBILITY_CHANGE_TIMEOUT + // If the user has come back to the page within SESSION_IDLE_DURATION // ms, we will re-use the existing session, otherwise create a new // session __DEBUG_BUILD__ && logger.log('[Replay] Document has become active, but session has expired'); diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 5ba806734cae..3eb2288a31fb 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -5,7 +5,7 @@ import { ERROR_CHECKOUT_TIME, MAX_SESSION_LIFE, REPLAY_SESSION_KEY, - VISIBILITY_CHANGE_TIMEOUT, + SESSION_IDLE_DURATION, WINDOW, } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; @@ -154,7 +154,7 @@ describe('Integration | errorSampleRate', () => { }); }); - it('does not send a replay when triggering a full dom snapshot when document becomes visible after [VISIBILITY_CHANGE_TIMEOUT]ms', async () => { + it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_DURATION]ms', async () => { Object.defineProperty(document, 'visibilityState', { configurable: true, get: function () { @@ -162,7 +162,7 @@ describe('Integration | errorSampleRate', () => { }, }); - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT + 1); + jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); document.dispatchEvent(new Event('visibilitychange')); @@ -186,8 +186,8 @@ describe('Integration | errorSampleRate', () => { expect(replay).not.toHaveLastSentReplay(); - // User comes back before `VISIBILITY_CHANGE_TIMEOUT` elapses - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT - 100); + // User comes back before `SESSION_IDLE_DURATION` elapses + jest.advanceTimersByTime(SESSION_IDLE_DURATION - 100); Object.defineProperty(document, 'visibilityState', { configurable: true, get: function () { diff --git a/packages/replay/test/integration/events.test.ts b/packages/replay/test/integration/events.test.ts index 45938fbc7fa6..57c49ba1e245 100644 --- a/packages/replay/test/integration/events.test.ts +++ b/packages/replay/test/integration/events.test.ts @@ -40,7 +40,7 @@ describe('Integration | events', () => { // Create a new session and clear mocks because a segment (from initial // checkout) will have already been uploaded by the time the tests run clearSession(replay); - replay['_loadAndCheckSession'](0); + replay['_loadAndCheckSession'](); mockTransportSend.mockClear(); }); @@ -93,7 +93,7 @@ describe('Integration | events', () => { it('has correct timestamps when there are events earlier than initial timestamp', async function () { clearSession(replay); - replay['_loadAndCheckSession'](0); + replay['_loadAndCheckSession'](); mockTransportSend.mockClear(); Object.defineProperty(document, 'visibilityState', { configurable: true, diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index 2398c22e2f8a..5d91edf483a3 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -1,6 +1,6 @@ import * as SentryUtils from '@sentry/utils'; -import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; +import { DEFAULT_FLUSH_MIN_DELAY, WINDOW } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; import type { EventBuffer } from '../../src/types'; import * as AddMemoryEntry from '../../src/util/addMemoryEntry'; @@ -95,7 +95,7 @@ describe('Integration | flush', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); clearSession(replay); - replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); + replay['_loadAndCheckSession'](); mockRecord.takeFullSnapshot.mockClear(); Object.defineProperty(WINDOW, 'location', { value: prevLocation, diff --git a/packages/replay/test/integration/rateLimiting.test.ts b/packages/replay/test/integration/rateLimiting.test.ts index ad0d6cfbdb22..fcd170b31784 100644 --- a/packages/replay/test/integration/rateLimiting.test.ts +++ b/packages/replay/test/integration/rateLimiting.test.ts @@ -1,7 +1,7 @@ import { getCurrentHub } from '@sentry/core'; import type { Transport } from '@sentry/types'; -import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION } from '../../src/constants'; +import { DEFAULT_FLUSH_MIN_DELAY } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; import * as SendReplayRequest from '../../src/util/sendReplayRequest'; import { BASE_TIMESTAMP, mockSdk } from '../index'; @@ -46,7 +46,7 @@ describe('Integration | rate-limiting behaviour', () => { // Create a new session and clear mocks because a segment (from initial // checkout) will have already been uploaded by the time the tests run clearSession(replay); - replay['_loadAndCheckSession'](0); + replay['_loadAndCheckSession'](); mockSendReplayRequest.mockClear(); }); @@ -57,7 +57,7 @@ describe('Integration | rate-limiting behaviour', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); clearSession(replay); jest.clearAllMocks(); - replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); + replay['_loadAndCheckSession'](); }); afterAll(() => { diff --git a/packages/replay/test/integration/sendReplayEvent.test.ts b/packages/replay/test/integration/sendReplayEvent.test.ts index 3263e1a09772..4f17644a241e 100644 --- a/packages/replay/test/integration/sendReplayEvent.test.ts +++ b/packages/replay/test/integration/sendReplayEvent.test.ts @@ -2,7 +2,7 @@ import * as SentryCore from '@sentry/core'; import type { Transport } from '@sentry/types'; import * as SentryUtils from '@sentry/utils'; -import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; +import { DEFAULT_FLUSH_MIN_DELAY, WINDOW } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; import { addEvent } from '../../src/util/addEvent'; import * as SendReplayRequest from '../../src/util/sendReplayRequest'; @@ -59,7 +59,7 @@ describe('Integration | sendReplayEvent', () => { // Create a new session and clear mocks because a segment (from initial // checkout) will have already been uploaded by the time the tests run clearSession(replay); - replay['_loadAndCheckSession'](0); + replay['_loadAndCheckSession'](); mockSendReplayRequest.mockClear(); }); @@ -69,7 +69,7 @@ describe('Integration | sendReplayEvent', () => { await new Promise(process.nextTick); jest.setSystemTime(new Date(BASE_TIMESTAMP)); clearSession(replay); - replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); + replay['_loadAndCheckSession'](); }); afterAll(() => { diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index dc54503e922d..d6820db61da7 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -5,7 +5,7 @@ import { DEFAULT_FLUSH_MIN_DELAY, MAX_SESSION_LIFE, REPLAY_SESSION_KEY, - VISIBILITY_CHANGE_TIMEOUT, + SESSION_IDLE_DURATION, WINDOW, } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; @@ -55,7 +55,7 @@ describe('Integration | session', () => { }); }); - it('creates a new session and triggers a full dom snapshot when document becomes visible after [VISIBILITY_CHANGE_TIMEOUT]ms', () => { + 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 () { @@ -65,7 +65,7 @@ describe('Integration | session', () => { const initialSession = replay.session; - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT + 1); + jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); document.dispatchEvent(new Event('visibilitychange')); @@ -75,7 +75,7 @@ describe('Integration | session', () => { expect(replay).not.toHaveSameSession(initialSession); }); - it('does not create a new session if user hides the tab and comes back within [VISIBILITY_CHANGE_TIMEOUT] seconds', () => { + 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', { @@ -88,8 +88,8 @@ describe('Integration | session', () => { expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).toHaveSameSession(initialSession); - // User comes back before `VISIBILITY_CHANGE_TIMEOUT` elapses - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT - 1); + // User comes back before `SESSION_IDLE_DURATION` elapses + jest.advanceTimersByTime(SESSION_IDLE_DURATION - 1); Object.defineProperty(document, 'visibilityState', { configurable: true, get: function () { @@ -184,7 +184,7 @@ describe('Integration | session', () => { expect(replay.session).toBe(undefined); }); - it('creates a new session and triggers a full dom snapshot when document becomes visible after [VISIBILITY_CHANGE_TIMEOUT]ms', () => { + 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 () { @@ -194,7 +194,7 @@ describe('Integration | session', () => { const initialSession = replay.session; - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT + 1); + jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); document.dispatchEvent(new Event('visibilitychange')); @@ -204,7 +204,7 @@ describe('Integration | session', () => { expect(replay).not.toHaveSameSession(initialSession); }); - it('creates a new session and triggers a full dom snapshot when document becomes focused after [VISIBILITY_CHANGE_TIMEOUT]ms', () => { + 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 () { @@ -214,7 +214,7 @@ describe('Integration | session', () => { const initialSession = replay.session; - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT + 1); + jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); WINDOW.dispatchEvent(new Event('focus')); @@ -224,7 +224,7 @@ describe('Integration | session', () => { expect(replay).not.toHaveSameSession(initialSession); }); - it('does not create a new session if user hides the tab and comes back within [VISIBILITY_CHANGE_TIMEOUT] seconds', () => { + 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', { @@ -237,8 +237,8 @@ describe('Integration | session', () => { expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).toHaveSameSession(initialSession); - // User comes back before `VISIBILITY_CHANGE_TIMEOUT` elapses - jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT - 1); + // User comes back before `SESSION_IDLE_DURATION` elapses + jest.advanceTimersByTime(SESSION_IDLE_DURATION - 1); Object.defineProperty(document, 'visibilityState', { configurable: true, get: function () { @@ -451,7 +451,7 @@ describe('Integration | session', () => { it('increases segment id after each event', async () => { clearSession(replay); - replay['_loadAndCheckSession'](0); + replay['_loadAndCheckSession'](); Object.defineProperty(document, 'visibilityState', { configurable: true, diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index 7e091bc7b49b..05772ecab6c7 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -1,7 +1,7 @@ import * as SentryUtils from '@sentry/utils'; import type { Replay } from '../../src'; -import { SESSION_IDLE_DURATION, WINDOW } from '../../src/constants'; +import { WINDOW } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; import { addEvent } from '../../src/util/addEvent'; // mock functions need to be imported first @@ -44,7 +44,7 @@ describe('Integration | stop', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); clearSession(replay); - replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); + replay['_loadAndCheckSession'](); mockRecord.takeFullSnapshot.mockClear(); mockAddInstrumentationHandler.mockClear(); Object.defineProperty(WINDOW, 'location', { diff --git a/packages/replay/test/utils/setupReplayContainer.ts b/packages/replay/test/utils/setupReplayContainer.ts index 15eaa47d5736..9a9455a3728a 100644 --- a/packages/replay/test/utils/setupReplayContainer.ts +++ b/packages/replay/test/utils/setupReplayContainer.ts @@ -1,4 +1,3 @@ -import { SESSION_IDLE_DURATION } from '../../src/constants'; import { createEventBuffer } from '../../src/eventBuffer'; import { ReplayContainer } from '../../src/replay'; import type { RecordingOptions, ReplayPluginOptions } from '../../src/types'; @@ -28,7 +27,7 @@ export function setupReplayContainer({ clearSession(replay); replay['_setInitialState'](); - replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); + replay['_loadAndCheckSession'](); replay['_isEnabled'] = true; replay.eventBuffer = createEventBuffer({ useCompression: options?.useCompression || false,