diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 651d430e6b0d..2c648e13b786 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -155,7 +155,9 @@ export class ReplayContainer implements ReplayContainerInterface { public start(): void { this._setInitialState(); - this._loadSession({ expiry: SESSION_IDLE_DURATION }); + if (!this._loadAndCheckSession()) { + return; + } // If there is no session, then something bad has happened - can't continue if (!this.session) { @@ -234,6 +236,10 @@ export class ReplayContainer implements ReplayContainerInterface { * does not support a teardown */ public stop(): void { + if (!this._isEnabled) { + return; + } + try { __DEBUG_BUILD__ && logger.log('[Replay] Stopping Replays'); this._isEnabled = false; @@ -264,6 +270,10 @@ export class ReplayContainer implements ReplayContainerInterface { * new DOM checkout.` */ public resume(): void { + if (!this._loadAndCheckSession()) { + return; + } + this._isPaused = false; this.startRecording(); } @@ -310,7 +320,9 @@ export class ReplayContainer implements ReplayContainerInterface { if (!this._stopRecording) { // 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 }); + if (!this._loadAndCheckSession()) { + return; + } // Note: This will cause a new DOM checkout this.resume(); @@ -348,7 +360,7 @@ export class ReplayContainer implements ReplayContainerInterface { * Returns true if session is not expired, false otherwise. * @hidden */ - public checkAndHandleExpiredSession({ expiry = SESSION_IDLE_DURATION }: { expiry?: number } = {}): boolean | void { + public checkAndHandleExpiredSession(expiry?: number): boolean | void { const oldSessionId = this.getSessionId(); // Prevent starting a new session if the last user activity is older than @@ -363,7 +375,9 @@ export class ReplayContainer implements ReplayContainerInterface { // --- There is recent user activity --- // // This will create a new session if expired, based on expiry length - this._loadSession({ expiry }); + if (!this._loadAndCheckSession(expiry)) { + return; + } // Session was expired if session ids do not match const expired = oldSessionId !== this.getSessionId(); @@ -388,10 +402,10 @@ export class ReplayContainer implements ReplayContainerInterface { } /** - * Loads a session from storage, or creates a new one if it does not exist or - * is expired. + * Loads (or refreshes) the current session. + * Returns false if session is not recorded. */ - private _loadSession({ expiry }: { expiry: number }): void { + private _loadAndCheckSession(expiry = SESSION_IDLE_DURATION): boolean { const { type, session } = getSession({ expiry, stickySession: Boolean(this._options.stickySession), @@ -412,6 +426,13 @@ export class ReplayContainer implements ReplayContainerInterface { } this.session = session; + + if (!this.session.sampled) { + this.stop(); + return false; + } + + return true; } /** @@ -618,9 +639,7 @@ export class ReplayContainer implements ReplayContainerInterface { return; } - const isSessionActive = this.checkAndHandleExpiredSession({ - expiry: VISIBILITY_CHANGE_TIMEOUT, - }); + const isSessionActive = this.checkAndHandleExpiredSession(VISIBILITY_CHANGE_TIMEOUT); if (!isSessionActive) { // If the user has come back to the page within VISIBILITY_CHANGE_TIMEOUT diff --git a/packages/replay/src/session/getSession.ts b/packages/replay/src/session/getSession.ts index 254fd33866c8..54b16d9a5414 100644 --- a/packages/replay/src/session/getSession.ts +++ b/packages/replay/src/session/getSession.ts @@ -4,6 +4,7 @@ import type { Session, SessionOptions } from '../types'; import { isSessionExpired } from '../util/isSessionExpired'; import { createSession } from './createSession'; import { fetchSession } from './fetchSession'; +import { makeSession } from './Session'; interface GetSessionParams extends SessionOptions { /** @@ -38,6 +39,10 @@ export function getSession({ if (!isExpired) { return { type: 'saved', session }; + } else if (session.sampled === 'error') { + // Error samples should not be re-created when expired, but instead we stop when the replay is done + const discardedSession = makeSession({ sampled: false }); + return { type: 'new', session: discardedSession }; } else { __DEBUG_BUILD__ && logger.log('[Replay] Session has expired'); } diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index e95b2b1594b9..b58805e477bf 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -1,8 +1,9 @@ -import { captureException } from '@sentry/core'; +import { captureException, getCurrentHub } from '@sentry/core'; import { DEFAULT_FLUSH_MIN_DELAY, ERROR_CHECKOUT_TIME, + MAX_SESSION_LIFE, REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW, @@ -264,12 +265,10 @@ describe('Integration | errorSampleRate', () => { expect(replay).not.toHaveLastSentReplay(); }); - it('does not upload if user has been idle for more than 15 minutes and comes back to move their mouse', async () => { + it('stops replay if user has been idle for more than 15 minutes and comes back to move their mouse', async () => { // Idle for 15 minutes jest.advanceTimersByTime(15 * 60000); - // 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, @@ -281,15 +280,11 @@ describe('Integration | errorSampleRate', () => { jest.runAllTimers(); 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. + // We stop recording after 15 minutes of inactivity in error mode expect(replay).not.toHaveLastSentReplay(); - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledWith(true); + expect(replay.isEnabled()).toBe(false); + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); }); it('has the correct timestamps with deferred root event and last replay update', async () => { @@ -375,6 +370,52 @@ describe('Integration | errorSampleRate', () => { ]), }); }); + + it('stops replay when session expires', 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(); + + // Wait a bit, shortly before session expires + jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + replay.triggerUserActivity(); + + expect(replay).toHaveLastSentReplay(); + + // Now wait after session expires - should stop recording + mockRecord.takeFullSnapshot.mockClear(); + (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); + + jest.advanceTimersByTime(10_000); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + replay.triggerUserActivity(); + + 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); + }); }); /** diff --git a/packages/replay/test/integration/events.test.ts b/packages/replay/test/integration/events.test.ts index 5168426dfd79..ce6978ed21b2 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['_loadSession']({ expiry: 0 }); + replay['_loadAndCheckSession'](0); 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['_loadSession']({ expiry: 0 }); + replay['_loadAndCheckSession'](0); 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 c0c143b255e4..2398c22e2f8a 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -95,7 +95,7 @@ describe('Integration | flush', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); clearSession(replay); - replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); + replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); 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 31e15638c314..379efb9b430d 100644 --- a/packages/replay/test/integration/rateLimiting.test.ts +++ b/packages/replay/test/integration/rateLimiting.test.ts @@ -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['_loadSession']({ expiry: 0 }); + replay['_loadAndCheckSession'](0); mockSendReplayRequest.mockClear(); }); @@ -57,7 +57,7 @@ describe('Integration | rate-limiting behaviour', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); clearSession(replay); jest.clearAllMocks(); - replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); + replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); }); afterAll(() => { diff --git a/packages/replay/test/integration/sendReplayEvent.test.ts b/packages/replay/test/integration/sendReplayEvent.test.ts index 0ee456729676..2a9130e7bb45 100644 --- a/packages/replay/test/integration/sendReplayEvent.test.ts +++ b/packages/replay/test/integration/sendReplayEvent.test.ts @@ -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['_loadSession']({ expiry: 0 }); + replay['_loadAndCheckSession'](0); mockSendReplayRequest.mockClear(); }); @@ -69,7 +69,7 @@ describe('Integration | sendReplayEvent', () => { await new Promise(process.nextTick); jest.setSystemTime(new Date(BASE_TIMESTAMP)); clearSession(replay); - replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); + replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); }); afterAll(() => { diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index 0a8a79dc5467..dc54503e922d 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -451,7 +451,7 @@ describe('Integration | session', () => { it('increases segment id after each event', async () => { clearSession(replay); - replay['_loadSession']({ expiry: 0 }); + replay['_loadAndCheckSession'](0); 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 55f8dafd9289..651a21f7e5bc 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -44,7 +44,7 @@ describe('Integration | stop', () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); sessionStorage.clear(); clearSession(replay); - replay['_loadSession']({ expiry: SESSION_IDLE_DURATION }); + replay['_loadAndCheckSession'](SESSION_IDLE_DURATION); mockRecord.takeFullSnapshot.mockClear(); mockAddInstrumentationHandler.mockClear(); Object.defineProperty(WINDOW, 'location', {