diff --git a/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/init.js b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/init.js new file mode 100644 index 000000000000..f9dccbffb530 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/init.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = Sentry.replayIntegration({ + flushMinDelay: 200, + flushMaxDelay: 200, + minReplayDuration: 0, + stickySession: true, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + + integrations: [window.Replay], +}); diff --git a/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/subject.js b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/subject.js new file mode 100644 index 000000000000..1c9b22455261 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/subject.js @@ -0,0 +1,11 @@ +document.getElementById('error1').addEventListener('click', () => { + throw new Error('First Error'); +}); + +document.getElementById('error2').addEventListener('click', () => { + throw new Error('Second Error'); +}); + +document.getElementById('click').addEventListener('click', () => { + // Just a click for interaction +}); diff --git a/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/template.html b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/template.html new file mode 100644 index 000000000000..1beb4b281b28 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/template.html @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/test.ts b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/test.ts new file mode 100644 index 000000000000..48173b3cac2a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/test.ts @@ -0,0 +1,330 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; +import { + getReplaySnapshot, + isReplayEvent, + shouldSkipReplayTest, + waitForReplayRunning, +} from '../../../utils/replayHelpers'; + +sentryTest( + 'buffer mode remains after interrupting error event ingest', + async ({ getLocalTestUrl, page, browserName }) => { + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + let errorCount = 0; + let replayCount = 0; + const errorEventIds: string[] = []; + const replayIds: string[] = []; + let firstReplayEventResolved: (value?: unknown) => void = () => {}; + // Need TS 5.7 for withResolvers + const firstReplayEventPromise = new Promise(resolve => { + firstReplayEventResolved = resolve; + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + + await page.route('https://dsn.ingest.sentry.io/**/*', async route => { + const event = envelopeRequestParser(route.request()); + + // Track error events + if (event && !event.type && event.event_id) { + errorCount++; + errorEventIds.push(event.event_id); + if (event.tags?.replayId) { + replayIds.push(event.tags.replayId as string); + + if (errorCount === 1) { + firstReplayEventResolved(); + // intentional so that it never resolves, we'll force a reload instead to interrupt the normal flow + await new Promise(resolve => setTimeout(resolve, 100000)); + } + } + } + + // Track replay events and simulate failure for the first replay + if (event && isReplayEvent(event)) { + replayCount++; + } + + // Success for other requests + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + await page.goto(url); + + // Wait for replay to initialize + await waitForReplayRunning(page); + + waitForErrorRequest(page); + await page.locator('#error1').click(); + + // This resolves, but the route doesn't get fulfilled as we want the reload to "interrupt" this flow + await firstReplayEventPromise; + expect(errorCount).toBe(1); + expect(replayCount).toBe(0); + expect(replayIds).toHaveLength(1); + + const firstSession = await getReplaySnapshot(page); + const firstSessionId = firstSession.session?.id; + expect(firstSessionId).toBeDefined(); + expect(firstSession.session?.sampled).toBe('buffer'); + expect(firstSession.session?.dirty).toBe(true); + expect(firstSession.recordingMode).toBe('buffer'); + + await page.reload(); + const secondSession = await getReplaySnapshot(page); + expect(secondSession.session?.sampled).toBe('buffer'); + expect(secondSession.session?.dirty).toBe(true); + expect(secondSession.recordingMode).toBe('buffer'); + expect(secondSession.session?.id).toBe(firstSessionId); + expect(secondSession.session?.segmentId).toBe(0); + }, +); + +sentryTest.only( + 'buffer mode remains after interrupting replay flush', + async ({ getLocalTestUrl, page, browserName }) => { + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + let errorCount = 0; + let replayCount = 0; + const errorEventIds: string[] = []; + const replayIds: string[] = []; + let firstReplayEventResolved: (value?: unknown) => void = () => {}; + // Need TS 5.7 for withResolvers + const firstReplayEventPromise = new Promise(resolve => { + firstReplayEventResolved = resolve; + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + + await page.route('https://dsn.ingest.sentry.io/**/*', async route => { + const event = envelopeRequestParser(route.request()); + + // Track error events + if (event && !event.type && event.event_id) { + errorCount++; + errorEventIds.push(event.event_id); + if (event.tags?.replayId) { + replayIds.push(event.tags.replayId as string); + } + } + + // Track replay events and simulate failure for the first replay + if (event && isReplayEvent(event)) { + replayCount++; + if (replayCount === 1) { + firstReplayEventResolved(); + // intentional so that it never resolves, we'll force a reload instead to interrupt the normal flow + await new Promise(resolve => setTimeout(resolve, 100000)); + } + } + + // Success for other requests + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + await page.goto(url); + + // Wait for replay to initialize + await waitForReplayRunning(page); + + await page.locator('#error1').click(); + await firstReplayEventPromise; + expect(errorCount).toBe(1); + expect(replayCount).toBe(1); + expect(replayIds).toHaveLength(1); + + // Get the first session info + const firstSession = await getReplaySnapshot(page); + const firstSessionId = firstSession.session?.id; + expect(firstSessionId).toBeDefined(); + expect(firstSession.session?.sampled).toBe('buffer'); + expect(firstSession.session?.dirty).toBe(true); + expect(firstSession.recordingMode).toBe('buffer'); // But still in buffer mode + + await page.reload(); + await waitForReplayRunning(page); + const secondSession = await getReplaySnapshot(page); + expect(secondSession.session?.sampled).toBe('buffer'); + expect(secondSession.session?.dirty).toBe(true); + expect(secondSession.session?.id).toBe(firstSessionId); + expect(secondSession.session?.segmentId).toBe(1); + // Because a flush attempt was made and not allowed to complete, segmentId increased from 0, + // so we resume in session mode + expect(secondSession.recordingMode).toBe('session'); + }, +); + +sentryTest( + 'starts a new session after interrupting replay flush and session "expires"', + async ({ getLocalTestUrl, page, browserName }) => { + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + let errorCount = 0; + let replayCount = 0; + const errorEventIds: string[] = []; + const replayIds: string[] = []; + let firstReplayEventResolved: (value?: unknown) => void = () => {}; + // Need TS 5.7 for withResolvers + const firstReplayEventPromise = new Promise(resolve => { + firstReplayEventResolved = resolve; + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + + await page.route('https://dsn.ingest.sentry.io/**/*', async route => { + const event = envelopeRequestParser(route.request()); + + // Track error events + if (event && !event.type && event.event_id) { + errorCount++; + errorEventIds.push(event.event_id); + if (event.tags?.replayId) { + replayIds.push(event.tags.replayId as string); + } + } + + // Track replay events and simulate failure for the first replay + if (event && isReplayEvent(event)) { + replayCount++; + if (replayCount === 1) { + firstReplayEventResolved(); + // intentional so that it never resolves, we'll force a reload instead to interrupt the normal flow + await new Promise(resolve => setTimeout(resolve, 100000)); + } + } + + // Success for other requests + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + await page.goto(url); + + // Wait for replay to initialize + await waitForReplayRunning(page); + + // Trigger first error - this should change session sampled to "session" + await page.locator('#error1').click(); + await firstReplayEventPromise; + expect(errorCount).toBe(1); + expect(replayCount).toBe(1); + expect(replayIds).toHaveLength(1); + + // Get the first session info + const firstSession = await getReplaySnapshot(page); + const firstSessionId = firstSession.session?.id; + expect(firstSessionId).toBeDefined(); + expect(firstSession.session?.sampled).toBe('buffer'); + expect(firstSession.session?.dirty).toBe(true); + expect(firstSession.recordingMode).toBe('buffer'); // But still in buffer mode + + // Now expire the session by manipulating session storage + // Simulate session expiry by setting lastActivity to a time in the past + await page.evaluate(() => { + const replayIntegration = (window as any).Replay; + const replay = replayIntegration['_replay']; + + // Set session as expired (15 minutes ago) + if (replay.session) { + const fifteenMinutesAgo = Date.now() - 15 * 60 * 1000; + replay.session.lastActivity = fifteenMinutesAgo; + replay.session.started = fifteenMinutesAgo; + + // Also update session storage if sticky sessions are enabled + const sessionKey = 'sentryReplaySession'; + const sessionData = sessionStorage.getItem(sessionKey); + if (sessionData) { + const session = JSON.parse(sessionData); + session.lastActivity = fifteenMinutesAgo; + session.started = fifteenMinutesAgo; + sessionStorage.setItem(sessionKey, JSON.stringify(session)); + } + } + }); + + await page.reload(); + const secondSession = await getReplaySnapshot(page); + expect(secondSession.session?.sampled).toBe('buffer'); + expect(secondSession.recordingMode).toBe('buffer'); + expect(secondSession.session?.id).not.toBe(firstSessionId); + expect(secondSession.session?.segmentId).toBe(0); + }, +); + +sentryTest( + 'marks session as dirty immediately when error is sampled in buffer mode', + async ({ getLocalTestUrl, page, browserName }) => { + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + await page.goto(url); + + // Wait for replay to initialize + await new Promise(resolve => setTimeout(resolve, 100)); + + // Verify initial state - buffer mode, not sampled + const initialSession = await getReplaySnapshot(page); + expect(initialSession.recordingMode).toBe('buffer'); + expect(initialSession.session?.sampled).toBe('buffer'); + expect(initialSession.session?.segmentId).toBe(0); + + // Trigger error which should immediately mark session as sampled + const reqErrorPromise = waitForErrorRequest(page); + await page.locator('#error1').click(); + + const duringErrorProcessing = await getReplaySnapshot(page); + expect(duringErrorProcessing.session?.sampled).toBe('buffer'); + expect(duringErrorProcessing.session?.dirty).toBe(true); + expect(duringErrorProcessing.recordingMode).toBe('buffer'); // Still in buffer recording mode + + await reqErrorPromise; + + // After error is sent, verify state is still correct + const afterError = await getReplaySnapshot(page); + expect(afterError.session?.sampled).toBe('buffer'); + expect(afterError.recordingMode).toBe('session'); + expect(afterError.session?.dirty).toBe(false); + + // Verify the session was persisted to sessionStorage (if sticky sessions enabled) + const sessionData = await page.evaluate(() => { + const sessionKey = 'sentryReplaySession'; + const data = sessionStorage.getItem(sessionKey); + return data ? JSON.parse(data) : null; + }); + + expect(sessionData).toBeDefined(); + expect(sessionData.sampled).toBe('buffer'); + expect(sessionData.dirty).toBe(false); + }, +); diff --git a/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts b/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts index 55559c0d4c01..b28d4547265e 100644 --- a/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts @@ -1,5 +1,6 @@ import type { Event, EventHint } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; +import { saveSession } from '../session/saveSession'; import type { ReplayContainer } from '../types'; import { isErrorEvent, isFeedbackEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils'; import { isRrwebError } from '../util/isRrwebError'; @@ -69,6 +70,20 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even event.tags = { ...event.tags, replayId: replay.getSessionId() }; } + // If we sampled this error in buffer mode, immediately mark the session as "sampled" + // by changing the sampled state from 'buffer' to 'session'. Otherwise, if the application is interrupte + // before `afterSendEvent` occurs, then the session would remain as "buffer" but we have an error event + // that is tagged with a replay id. This could end up creating replays w/ excessive durations because + // of the linked error. + if (isErrorEventSampled && replay.recordingMode === 'buffer' && replay.session?.sampled === 'buffer') { + const session = replay.session; + session.dirty = true; + // Save the session if sticky sessions are enabled to persist the state change + if (replay.getOptions().stickySession) { + saveSession(session); + } + } + return event; }, { id: 'Replay' }, diff --git a/packages/replay-internal/src/replay.ts b/packages/replay-internal/src/replay.ts index 61676f790b4d..49e8ce092edd 100644 --- a/packages/replay-internal/src/replay.ts +++ b/packages/replay-internal/src/replay.ts @@ -606,6 +606,7 @@ export class ReplayContainer implements ReplayContainerInterface { // Once this session ends, we do not want to refresh it if (this.session) { + this.session.dirty = false; this._updateUserActivity(activityTime); this._updateSessionActivity(activityTime); this._maybeSaveSession(); diff --git a/packages/replay-internal/src/session/Session.ts b/packages/replay-internal/src/session/Session.ts index 554f625cc8e9..59e6b09ed43c 100644 --- a/packages/replay-internal/src/session/Session.ts +++ b/packages/replay-internal/src/session/Session.ts @@ -13,6 +13,7 @@ export function makeSession(session: Partial & { sampled: Sampled }): S const segmentId = session.segmentId || 0; const sampled = session.sampled; const previousSessionId = session.previousSessionId; + const dirty = session.dirty || false; return { id, @@ -21,5 +22,6 @@ export function makeSession(session: Partial & { sampled: Sampled }): S segmentId, sampled, previousSessionId, + dirty, }; } diff --git a/packages/replay-internal/src/types/replay.ts b/packages/replay-internal/src/types/replay.ts index 1e7891a84e76..a2c84d6c4bbe 100644 --- a/packages/replay-internal/src/types/replay.ts +++ b/packages/replay-internal/src/types/replay.ts @@ -383,6 +383,13 @@ export interface Session { * Is the session sampled? `false` if not sampled, otherwise, `session` or `buffer` */ sampled: Sampled; + + /** + * Session is dirty when its id has been linked to an event (e.g. error event). + * This is helpful when a session is mistakenly stuck in "buffer" mode (e.g. network issues preventing it from being converted to "session" mode). + * The dirty flag is used to prevent updating the session start time to the earliest event in the buffer so that it can be refreshed if it's been expired. + */ + dirty?: boolean; } export type EventBufferType = 'sync' | 'worker'; diff --git a/packages/replay-internal/src/util/handleRecordingEmit.ts b/packages/replay-internal/src/util/handleRecordingEmit.ts index 0ae87601637b..aeb49f0cd259 100644 --- a/packages/replay-internal/src/util/handleRecordingEmit.ts +++ b/packages/replay-internal/src/util/handleRecordingEmit.ts @@ -72,7 +72,7 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa // When in buffer mode, make sure we adjust the session started date to the current earliest event of the buffer // this should usually be the timestamp of the checkout event, but to be safe... - if (replay.recordingMode === 'buffer' && session && replay.eventBuffer) { + if (replay.recordingMode === 'buffer' && session && replay.eventBuffer && !session.dirty) { const earliestEvent = replay.eventBuffer.getEarliestTimestamp(); if (earliestEvent) { DEBUG_BUILD && diff --git a/packages/replay-internal/test/integration/errorSampleRate.test.ts b/packages/replay-internal/test/integration/errorSampleRate.test.ts index f79e393df7e3..b0f1dec8f0e0 100644 --- a/packages/replay-internal/test/integration/errorSampleRate.test.ts +++ b/packages/replay-internal/test/integration/errorSampleRate.test.ts @@ -80,6 +80,8 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); + await vi.advanceTimersToNextTimerAsync(); + // need 2nd tick to wait for `saveSession` to complete in `handleGlobalEvents await vi.advanceTimersToNextTimerAsync(); expect(replay).toHaveLastSentReplay({ @@ -158,6 +160,7 @@ describe('Integration | errorSampleRate', () => { segmentId: 0, sampled: 'buffer', previousSessionId: 'previoussessionid', + dirty: false, }), })); @@ -179,6 +182,8 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); await vi.advanceTimersToNextTimerAsync(); + // need 2nd tick to wait for `saveSession` to complete in `handleGlobalEvents + await vi.advanceTimersToNextTimerAsync(); // Converts to session mode expect(replay.recordingMode).toBe('session'); @@ -508,6 +513,8 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); + await vi.advanceTimersToNextTimerAsync(); + // need 2nd tick to wait for `saveSession` to complete in `handleGlobalEvents await vi.advanceTimersToNextTimerAsync(); expect(replay).toHaveLastSentReplay({ @@ -604,6 +611,8 @@ describe('Integration | errorSampleRate', () => { // should still react to errors later on captureException(new Error('testing')); + await vi.advanceTimersToNextTimerAsync(); + // need 2nd tick to wait for `saveSession` to complete in `handleGlobalEvents await vi.advanceTimersToNextTimerAsync(); expect(replay.session?.id).toBe(oldSessionId); @@ -739,7 +748,8 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); await vi.advanceTimersToNextTimerAsync(); - // await vi.advanceTimersToNextTimerAsync(); + // need 2nd tick to wait for `saveSession` to complete in `handleGlobalEvents + await vi.advanceTimersToNextTimerAsync(); // This is still the timestamp from the full snapshot we took earlier expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED); diff --git a/packages/replay-internal/test/unit/session/fetchSession.test.ts b/packages/replay-internal/test/unit/session/fetchSession.test.ts index 46f0f05f5c9a..9dee5cb5cee0 100644 --- a/packages/replay-internal/test/unit/session/fetchSession.test.ts +++ b/packages/replay-internal/test/unit/session/fetchSession.test.ts @@ -28,6 +28,7 @@ describe('Unit | session | fetchSession', () => { ); expect(fetchSession()).toEqual({ + dirty: false, id: 'fd09adfc4117477abc8de643e5a5798a', lastActivity: 1648827162658, segmentId: 0, @@ -39,10 +40,11 @@ describe('Unit | session | fetchSession', () => { it('fetches an unsampled session', function () { WINDOW.sessionStorage.setItem( REPLAY_SESSION_KEY, - '{"id":"fd09adfc4117477abc8de643e5a5798a","sampled": false,"started":1648827162630,"lastActivity":1648827162658}', + '{"id":"fd09adfc4117477abc8de643e5a5798a","sampled": false,"started":1648827162630,"lastActivity":1648827162658,"dirty":true}', ); expect(fetchSession()).toEqual({ + dirty: true, id: 'fd09adfc4117477abc8de643e5a5798a', lastActivity: 1648827162658, segmentId: 0, diff --git a/packages/replay-internal/test/unit/session/loadOrCreateSession.test.ts b/packages/replay-internal/test/unit/session/loadOrCreateSession.test.ts index 273d401a7afc..dee44638344b 100644 --- a/packages/replay-internal/test/unit/session/loadOrCreateSession.test.ts +++ b/packages/replay-internal/test/unit/session/loadOrCreateSession.test.ts @@ -77,6 +77,7 @@ describe('Unit | session | loadOrCreateSession', () => { lastActivity: expect.any(Number), sampled: 'session', started: expect.any(Number), + dirty: false, }); // Should not have anything in storage @@ -104,6 +105,7 @@ describe('Unit | session | loadOrCreateSession', () => { lastActivity: expect.any(Number), sampled: 'session', started: expect.any(Number), + dirty: false, }); // Should not have anything in storage @@ -129,10 +131,10 @@ describe('Unit | session | loadOrCreateSession', () => { sampled: 'session', started: expect.any(Number), previousSessionId: 'previous_session_id', + dirty: false, }); }); }); - describe('stickySession: true', () => { it('creates new session if none exists', function () { const session = loadOrCreateSession( @@ -151,6 +153,7 @@ describe('Unit | session | loadOrCreateSession', () => { lastActivity: expect.any(Number), sampled: 'session', started: expect.any(Number), + dirty: false, }; expect(session).toEqual(expectedSession); @@ -181,6 +184,7 @@ describe('Unit | session | loadOrCreateSession', () => { sampled: 'session', started: expect.any(Number), previousSessionId: 'test_old_session_uuid', + dirty: false, }; expect(session).toEqual(expectedSession); expect(session.lastActivity).toBeGreaterThanOrEqual(now); @@ -209,6 +213,7 @@ describe('Unit | session | loadOrCreateSession', () => { lastActivity: date, sampled: 'session', started: date, + dirty: false, }); }); @@ -250,6 +255,7 @@ describe('Unit | session | loadOrCreateSession', () => { sampled: 'session', started: expect.any(Number), previousSessionId: 'previous_session_id', + dirty: false, }; expect(session).toEqual(expectedSession); @@ -347,6 +353,7 @@ describe('Unit | session | loadOrCreateSession', () => { segmentId: 0, lastActivity: expect.any(Number), sampled: false, + dirty: false, started: expect.any(Number), }; expect(session).toEqual(expectedSession);