From 53e2f3fa57d28632e8df485eb58e665706e04726 Mon Sep 17 00:00:00 2001 From: Fiona Date: Wed, 19 Aug 2026 08:16:48 -0700 Subject: [PATCH 01/27] test: point the profiling test helpers at the packages they actually export from mockProfiler imported registerCleanupTask and getGlobalObject from the rum package rather than core, and profiler.spec.ts imported from package names this repository does not publish. Since mockProfiler is re-exported from the rum test barrel, the broken imports took every spec that touches that barrel down with them - around 220 tests never ran. --- packages/rum/src/domain/profiling/profiler.spec.ts | 6 +++--- packages/rum/test/mockProfiler.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/rum/src/domain/profiling/profiler.spec.ts b/packages/rum/src/domain/profiling/profiler.spec.ts index eb10a48052..9c6cf38da6 100644 --- a/packages/rum/src/domain/profiling/profiler.spec.ts +++ b/packages/rum/src/domain/profiling/profiler.spec.ts @@ -1,6 +1,6 @@ -import { LifeCycle } from '@datadog/browser-rum-core' -import { relativeNow, timeStampNow } from '@datadog/browser-core' -import { setPageVisibility, restorePageVisibility, createNewEvent } from '@datadog/browser-core/test' +import { LifeCycle } from '@flashcatcloud/browser-rum-core' +import { relativeNow, timeStampNow } from '@flashcatcloud/browser-core' +import { setPageVisibility, restorePageVisibility, createNewEvent } from '@flashcatcloud/browser-core/test' import { createRumSessionManagerMock, mockPerformanceObserver, mockRumConfiguration } from '../../../../rum-core/test' import { mockProfiler } from '../../../test' import { mockedTrace } from './test-utils/mockedTrace' diff --git a/packages/rum/test/mockProfiler.ts b/packages/rum/test/mockProfiler.ts index fca2861ad3..d4db678e5b 100644 --- a/packages/rum/test/mockProfiler.ts +++ b/packages/rum/test/mockProfiler.ts @@ -1,5 +1,5 @@ -import { registerCleanupTask } from '@flashcatcloud/browser-rum/test' -import { getGlobalObject } from '@flashcatcloud/browser-rum' +import { registerCleanupTask } from '@flashcatcloud/browser-core/test' +import { getGlobalObject } from '@flashcatcloud/browser-core' import type { Profiler, ProfilerTrace, ProfilerInitOptions } from '../src/domain/profiling/types' export function mockProfiler(mockedTrace: ProfilerTrace) { From a40a5420032940f6d9e8b29ffa032b398f71f214 Mon Sep 17 00:00:00 2001 From: Fiona Date: Wed, 19 Aug 2026 08:16:58 -0700 Subject: [PATCH 02/27] feat(rum): add sessionReplayOnErrorSampleRate A session drawn by this rate records from the start but uploads nothing until it reports an error. If none ever happens, nothing is sent and the session is never stored. On the first error the withheld buffer is released and recording continues normally, so the replay covers what led up to the error rather than starting at it. The buffer is bounded on both axes. Time: a buffer that spans more than a minute is dropped and restarted from a fresh full snapshot, so what is released stays a minute at most. Size: the existing segment byte limit still applies while withheld, and restarts are spaced out so that a document whose full snapshot alone exceeds that limit degrades instead of restarting in a loop. A withheld buffer belongs to the session that produced it. It is released only when that same session reports the error - if the session expires or is renewed first, the records are dropped, so an expiry can never turn into an upload for a session that never errored. Buffers that are dropped roll back their replay stats, and has_replay is not reported while a replay is being withheld, so neither the counters nor the link offer a replay that does not exist. Errors raised by the SDK about its own transport do not release anything: those are our failures, not the application's, and counting them would make every session an error session wherever our endpoint is unreachable. --- .../core/src/domain/session/sessionManager.ts | 7 + packages/rum-core/src/boot/startRum.ts | 4 + .../configuration/configuration.spec.ts | 8 +- .../src/domain/configuration/configuration.ts | 21 +- .../src/domain/contexts/sessionContext.ts | 8 +- .../src/domain/rumSessionManager.spec.ts | 60 +++++ .../rum-core/src/domain/rumSessionManager.ts | 64 ++++- .../src/domain/trackSessionError.spec.ts | 77 ++++++ .../rum-core/src/domain/trackSessionError.ts | 44 ++++ .../rum-core/test/mockRumSessionManager.ts | 34 ++- packages/rum/src/boot/startRecording.ts | 26 +- .../rum/src/domain/getSessionReplayLink.ts | 5 + packages/rum/src/domain/record/record.ts | 8 +- .../src/domain/record/startFullSnapshots.ts | 14 ++ packages/rum/src/domain/replayStats.ts | 15 ++ .../segmentCollection.spec.ts | 230 ++++++++++++++++++ .../segmentCollection/segmentCollection.ts | 139 ++++++++++- 17 files changed, 725 insertions(+), 39 deletions(-) create mode 100644 packages/rum-core/src/domain/trackSessionError.spec.ts create mode 100644 packages/rum-core/src/domain/trackSessionError.ts diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 03caf2f49c..789d0d5487 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -27,6 +27,12 @@ export interface SessionContext extends Context { id: string trackingType: TrackingType isReplayForced: boolean + /** + * Whether an error has already been reported during this session. Persisted in the session store + * so it survives page navigation: an error session must not go back to withholding its replay + * just because the user moved to another page. + */ + hasError: boolean anonymousId: string | undefined } @@ -92,6 +98,7 @@ export function startSessionManager( id: sessionStore.getSession().id!, trackingType: sessionStore.getSession()[productKey] as TrackingType, isReplayForced: !!sessionStore.getSession().forcedReplay, + hasError: !!sessionStore.getSession().hasError, anonymousId: sessionStore.getSession().anonymousId, } } diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index e60741983b..0c1af632b9 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -28,6 +28,7 @@ import { startErrorCollection } from '../domain/error/errorCollection' import { startResourceCollection } from '../domain/resource/resourceCollection' import { startViewCollection } from '../domain/view/viewCollection' import { startRumSessionManager, startRumSessionManagerStub } from '../domain/rumSessionManager' +import { startSessionErrorTracking } from '../domain/trackSessionError' import { startRumBatch } from '../transport/startRumBatch' import { startRumEventBridge } from '../transport/startRumEventBridge' import { startUrlContexts } from '../domain/contexts/urlContexts' @@ -110,6 +111,9 @@ export function startRum( ? startRumSessionManager(configuration, lifeCycle, trackingConsentState) : startRumSessionManagerStub() + const sessionErrorTracking = startSessionErrorTracking(lifeCycle, session) + cleanupTasks.push(() => sessionErrorTracking.stop()) + if (!canUseEventBridge()) { const batch = startRumBatch( configuration, diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 0331d764bf..a2c4c48875 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -1,6 +1,9 @@ import type { InitConfiguration } from '@flashcatcloud/browser-core' import { DefaultPrivacyLevel, display, TraceContextInjection } from '@flashcatcloud/browser-core' -import { EXHAUSTIVE_INIT_CONFIGURATION, SERIALIZED_EXHAUSTIVE_INIT_CONFIGURATION } from '@flashcatcloud/browser-core/test' +import { + EXHAUSTIVE_INIT_CONFIGURATION, + SERIALIZED_EXHAUSTIVE_INIT_CONFIGURATION, +} from '@flashcatcloud/browser-core/test' import type { ExtractTelemetryConfiguration, CamelToSnakeCase, @@ -529,6 +532,7 @@ describe('serializeRumConfiguration', () => { enablePrivacyForActionName: false, subdomain: 'foo', sessionReplaySampleRate: 60, + sessionReplayOnErrorSampleRate: 40, startSessionReplayRecordingManually: true, trackUserInteractions: true, actionNameAttribute: 'test-id', @@ -554,6 +558,8 @@ describe('serializeRumConfiguration', () => { | 'remoteConfigurationId' | 'profilingSampleRate' | 'propagateTraceBaggage' + // not reported yet: needs a rum-events-format schema change first + | 'sessionReplayOnErrorSampleRate' ? never : CamelToSnakeCase // By specifying the type here, we can ensure that serializeConfiguration is returning an diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 8e25227ef5..33743b5d9e 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -100,6 +100,16 @@ export interface RumInitConfiguration extends InitConfiguration { * See [Configure Your Setup For Browser RUM and Browser RUM & Session Replay Sampling](https://docs.datadoghq.com/real_user_monitoring/guide/sampling-browser-plans) for further information. */ sessionReplaySampleRate?: number | undefined + /** + * The percentage of tracked sessions that record a replay but only upload it if the session + * reports an error: 100 for all, 0 for none. Drawn only for sessions that the plain + * `sessionReplaySampleRate` draw missed, so a session is never counted by both rates. + * + * Such a session records from the start and keeps at most the last minute of it in memory. If it + * never reports an error, nothing is uploaded and the session is not billed. On the first error, + * the withheld minute is uploaded and recording continues normally for the rest of the session. + */ + sessionReplayOnErrorSampleRate?: number | undefined /** * If the session is sampled for Session Replay, only start the recording when `startSessionReplayRecording()` is called, instead of at the beginning of the session. Default: if startSessionReplayRecording is 0, true; otherwise, false. * See [Session Replay Usage](https://docs.datadoghq.com/real_user_monitoring/session_replay/browser/#usage) for further information. @@ -175,6 +185,7 @@ export interface RumConfiguration extends Configuration { defaultPrivacyLevel: DefaultPrivacyLevel enablePrivacyForActionName: boolean sessionReplaySampleRate: number + sessionReplayOnErrorSampleRate: number startSessionReplayRecordingManually: boolean trackUserInteractions: boolean trackViewsManually: boolean @@ -207,6 +218,7 @@ export function validateAndBuildRumConfiguration( if ( !isSampleRate(initConfiguration.sessionReplaySampleRate, 'Session Replay') || + !isSampleRate(initConfiguration.sessionReplayOnErrorSampleRate, 'Session Replay on Error') || !isSampleRate(initConfiguration.traceSampleRate, 'Trace') ) { return @@ -230,16 +242,20 @@ export function validateAndBuildRumConfiguration( const profilingEnabled = isExperimentalFeatureEnabled(ExperimentalFeature.PROFILING) const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 + const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 return { applicationId: initConfiguration.applicationId, version: initConfiguration.version || undefined, actionNameAttribute: initConfiguration.actionNameAttribute, sessionReplaySampleRate, + sessionReplayOnErrorSampleRate, startSessionReplayRecordingManually: initConfiguration.startSessionReplayRecordingManually !== undefined ? !!initConfiguration.startSessionReplayRecordingManually - : sessionReplaySampleRate === 0, + : // An error-sampled session has to be recording before the error happens, otherwise there is + // nothing to withhold and release. So it must auto-start just like a plain sampled one. + sessionReplaySampleRate === 0 && sessionReplayOnErrorSampleRate === 0, traceSampleRate: initConfiguration.traceSampleRate ?? 100, rulePsr: isNumber(initConfiguration.traceSampleRate) ? initConfiguration.traceSampleRate / 100 : undefined, allowedTracingUrls, @@ -325,6 +341,9 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) { return { session_replay_sample_rate: configuration.sessionReplaySampleRate, + // `session_replay_on_error_sample_rate` is deliberately not reported yet: the telemetry + // configuration type is generated from the rum-events-format schema, so adding it needs a schema + // change first, and that is a separate repository. start_session_replay_recording_manually: configuration.startSessionReplayRecordingManually, trace_sample_rate: configuration.traceSampleRate, trace_context_injection: configuration.traceContextInjection, diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index a62a2da0ff..c8d893c110 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -20,15 +20,19 @@ export function startSessionContext( return DISCARDED } + // A session withholding its replay is recording, but nothing has been uploaded and nothing may + // ever be. Reporting `has_replay` here would offer a replay that does not exist. + const isReplayWithheld = session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR + let hasReplay let sampledForReplay let isActive if (eventType === RumEventType.VIEW) { - hasReplay = recorderApi.getReplayStats(view.id) ? true : undefined + hasReplay = !isReplayWithheld && recorderApi.getReplayStats(view.id) ? true : undefined sampledForReplay = session.sessionReplay === SessionReplayState.SAMPLED isActive = view.sessionIsActive ? undefined : false } else { - hasReplay = recorderApi.isRecording() ? true : undefined + hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined } return { diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index cb08bd0d1c..0c286da966 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -209,6 +209,66 @@ describe('rum session manager', () => { ) }) + describe('error session replay sampling', () => { + it('draws the error-replay type only when the plain replay draw missed', () => { + startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + }) + + it('stores the error-replay type when only that rate is hit', () => { + startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + ) + }) + + it('withholds the replay until the session reports an error', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) + + sessionManager.setSessionHasError() + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('keeps the released state across a page load, since it is persisted in the session store', () => { + setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=3&hasError=1', DURATION) + + const sessionManager = startRumSessionManagerWithDefaults() + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('releases the replay when it is forced, rather than waiting for an error that may never come', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) + + sessionManager.setForcedReplay() + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.FORCED) + }) + + it('tracks the session even when no replay rate is hit at all', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 0 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY) + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.OFF) + }) + }) + function startRumSessionManagerWithDefaults({ configuration }: { configuration?: Partial } = {}) { return startRumSessionManager( mockRumConfiguration({ diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 4d2f7829e7..e58383718a 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -24,6 +24,11 @@ export interface RumSessionManager { expire: () => void expireObservable: Observable setForcedReplay: () => void + /** + * Marks the session as having reported an error. For a session sampled by + * `sessionReplayOnErrorSampleRate`, this is what releases the withheld replay. + */ + setSessionHasError: () => void } export type RumSession = { @@ -36,12 +41,19 @@ export const enum RumTrackingType { NOT_TRACKED = '0', TRACKED_WITH_SESSION_REPLAY = '1', TRACKED_WITHOUT_SESSION_REPLAY = '2', + TRACKED_WITH_ERROR_SESSION_REPLAY = '3', } export const enum SessionReplayState { OFF, SAMPLED, FORCED, + /** + * The session records, but every segment is withheld until it reports its first error. If no error + * ever happens, nothing is uploaded and the session is never billed. Once an error is reported the + * session moves to `SAMPLED` and the withheld buffer is released. + */ + BUFFERED_ON_ERROR, } export function startRumSessionManager( @@ -71,6 +83,12 @@ export function startRumSessionManager( sessionEntity.isReplayForced = true } } + if (!previousState.hasError && newState.hasError) { + const sessionEntity = sessionManager.findSession() + if (sessionEntity) { + sessionEntity.hasError = true + } + } }) return { findTrackedSession: (startTime) => { @@ -80,19 +98,37 @@ export function startRumSessionManager( } return { id: session.id, - sessionReplay: - session.trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY - ? SessionReplayState.SAMPLED - : session.isReplayForced - ? SessionReplayState.FORCED - : SessionReplayState.OFF, + sessionReplay: computeSessionReplayState(session.trackingType, session.hasError, session.isReplayForced), anonymousId: session.anonymousId, } }, expire: sessionManager.expire, expireObservable: sessionManager.expireObservable, setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), + setSessionHasError: () => sessionManager.updateSessionState({ hasError: '1' }), + } +} + +export function computeSessionReplayState( + trackingType: RumTrackingType, + hasError: boolean, + isReplayForced: boolean +): SessionReplayState { + if (trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY) { + return SessionReplayState.SAMPLED } + if (trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY && hasError) { + return SessionReplayState.SAMPLED + } + // A forced replay wins over withholding: the host explicitly asked for this user's replay, so it + // must not keep waiting for an error that may never come. + if (isReplayForced) { + return SessionReplayState.FORCED + } + if (trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY) { + return SessionReplayState.BUFFERED_ON_ERROR + } + return SessionReplayState.OFF } /** @@ -108,6 +144,7 @@ export function startRumSessionManagerStub(): RumSessionManager { expire: noop, expireObservable: new Observable(), setForcedReplay: noop, + setSessionHasError: noop, } } @@ -117,10 +154,13 @@ function computeSessionState(configuration: RumConfiguration, rawTrackingType?: trackingType = rawTrackingType } else if (!performDraw(configuration.sessionSampleRate)) { trackingType = RumTrackingType.NOT_TRACKED - } else if (!performDraw(configuration.sessionReplaySampleRate)) { - trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY - } else { + } else if (performDraw(configuration.sessionReplaySampleRate)) { trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY + } else if (performDraw(configuration.sessionReplayOnErrorSampleRate)) { + // Drawn only when the plain replay draw missed, so a session is never counted by both rates. + trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + } else { + trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY } return { trackingType, @@ -132,13 +172,15 @@ function hasValidRumSession(trackingType?: string): trackingType is RumTrackingT return ( trackingType === RumTrackingType.NOT_TRACKED || trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || - trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY + trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || + trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY ) } function isTypeTracked(rumSessionType: RumTrackingType | undefined) { return ( rumSessionType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || - rumSessionType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY + rumSessionType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || + rumSessionType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY ) } diff --git a/packages/rum-core/src/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts new file mode 100644 index 0000000000..696413d97e --- /dev/null +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -0,0 +1,77 @@ +import type { Context } from '@flashcatcloud/browser-core' +import { registerCleanupTask } from '@flashcatcloud/browser-core/test' +import type { RumEvent } from '../rumEvent.types' +import { createRumSessionManagerMock } from '../../test' +import { LifeCycle, LifeCycleEventType } from './lifeCycle' +import { startSessionErrorTracking } from './trackSessionError' + +describe('startSessionErrorTracking', () => { + let lifeCycle: LifeCycle + let sessionManager: ReturnType + let setSessionHasErrorSpy: jasmine.Spy + + function collect(type: string, source = 'source') { + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type, error: { source } } as unknown as RumEvent & + Context) + } + + beforeEach(() => { + lifeCycle = new LifeCycle() + sessionManager = createRumSessionManagerMock() + setSessionHasErrorSpy = spyOn(sessionManager, 'setSessionHasError').and.callThrough() + const { stop } = startSessionErrorTracking(lifeCycle, sessionManager) + registerCleanupTask(stop) + }) + + it('marks the session on the first collected error', () => { + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('does not mark the session on other event types', () => { + collect('view') + collect('resource') + collect('action') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('ignores the SDK own failures, which are not the application reporting an error', () => { + collect('error', 'agent') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('still marks the session on a network error, which is the application reporting one', () => { + collect('error', 'network') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('marks the session only once, however many errors follow', () => { + collect('error') + collect('error') + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('marks a renewed session again, since it is a different session', () => { + collect('error') + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(2) + }) + + it('stops marking once stopped', () => { + const { stop } = startSessionErrorTracking(lifeCycle, sessionManager) + stop() + setSessionHasErrorSpy.calls.reset() + // the suite's own tracker is still running, so exactly one call is expected, not two + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts new file mode 100644 index 0000000000..e4b54fb058 --- /dev/null +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -0,0 +1,44 @@ +import { ErrorSource } from '@flashcatcloud/browser-core' +import { RumEventType } from '../rawRumEvent.types' +import type { LifeCycle } from './lifeCycle' +import { LifeCycleEventType } from './lifeCycle' +import type { RumSessionManager } from './rumSessionManager' + +/** + * Marks the session as having reported an error, which is what releases a replay withheld by + * `sessionReplayOnErrorSampleRate`. + * + * It listens after assembly rather than on the raw error, so an error discarded by `beforeSend` or + * by a rate limiter does not release anything: a session billed for an error that cannot be found + * afterwards would be worse than no replay at all. + */ +export function startSessionErrorTracking(lifeCycle: LifeCycle, sessionManager: RumSessionManager) { + let hasReportedError = false + + const eventSubscription = lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event) => { + if (hasReportedError || event.type !== RumEventType.ERROR) { + return + } + // The SDK's own failures — an intake request that could not be sent, for instance — are ours, + // not the application's. Counting them would turn every session into an error session for any + // customer whose network blocks our endpoint, billing them for replays of nothing. + if (event.error.source === ErrorSource.AGENT) { + return + } + hasReportedError = true + sessionManager.setSessionHasError() + }) + + // A renewed session is a different session: it draws its own sampling and starts out without an + // error, so anything withheld for it must stay withheld until it reports one of its own. + const renewSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { + hasReportedError = false + }) + + return { + stop: () => { + eventSubscription.unsubscribe() + renewSubscription.unsubscribe() + }, + } +} diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 6c43f9daec..9314b732a9 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -1,42 +1,46 @@ import { Observable } from '@flashcatcloud/browser-core' -import { SessionReplayState, type RumSessionManager } from '../src/domain/rumSessionManager' +import { RumTrackingType, computeSessionReplayState, type RumSessionManager } from '../src/domain/rumSessionManager' export interface RumSessionManagerMock extends RumSessionManager { setId(id: string): RumSessionManagerMock setNotTracked(): RumSessionManagerMock setTrackedWithoutSessionReplay(): RumSessionManagerMock setTrackedWithSessionReplay(): RumSessionManagerMock + setTrackedWithErrorSessionReplay(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock + setSessionHasError(): RumSessionManagerMock } const DEFAULT_ID = 'session-id' const enum SessionStatus { TRACKED_WITH_SESSION_REPLAY, TRACKED_WITHOUT_SESSION_REPLAY, + TRACKED_WITH_ERROR_SESSION_REPLAY, NOT_TRACKED, EXPIRED, } +const TRACKING_TYPES: { [key in SessionStatus]?: RumTrackingType } = { + [SessionStatus.TRACKED_WITH_SESSION_REPLAY]: RumTrackingType.TRACKED_WITH_SESSION_REPLAY, + [SessionStatus.TRACKED_WITHOUT_SESSION_REPLAY]: RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY, + [SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY]: RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY, +} + export function createRumSessionManagerMock(): RumSessionManagerMock { let id = DEFAULT_ID let sessionStatus: SessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY let forcedReplay: boolean = false + let hasError: boolean = false return { findTrackedSession() { - if ( - sessionStatus !== SessionStatus.TRACKED_WITH_SESSION_REPLAY && - sessionStatus !== SessionStatus.TRACKED_WITHOUT_SESSION_REPLAY - ) { + const trackingType = TRACKING_TYPES[sessionStatus] + if (!trackingType) { return undefined } return { id, - sessionReplay: - sessionStatus === SessionStatus.TRACKED_WITH_SESSION_REPLAY - ? SessionReplayState.SAMPLED - : forcedReplay - ? SessionReplayState.FORCED - : SessionReplayState.OFF, + // Derived the same way as in production, so the mock cannot drift from the real state machine + sessionReplay: computeSessionReplayState(trackingType, hasError, forcedReplay), anonymousId: 'device-123', } }, @@ -61,9 +65,17 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY return this }, + setTrackedWithErrorSessionReplay() { + sessionStatus = SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY + return this + }, setForcedReplay() { forcedReplay = true return this }, + setSessionHasError() { + hasError = true + return this + }, } } diff --git a/packages/rum/src/boot/startRecording.ts b/packages/rum/src/boot/startRecording.ts index b3bfff31e9..1cc4aba3c3 100644 --- a/packages/rum/src/boot/startRecording.ts +++ b/packages/rum/src/boot/startRecording.ts @@ -1,7 +1,7 @@ import type { RawError, HttpRequest, DeflateEncoder } from '@flashcatcloud/browser-core' -import { createHttpRequest, addTelemetryDebug, canUseEventBridge } from '@flashcatcloud/browser-core' +import { createHttpRequest, addTelemetryDebug, canUseEventBridge, noop } from '@flashcatcloud/browser-core' import type { LifeCycle, ViewHistory, RumConfiguration, RumSessionManager } from '@flashcatcloud/browser-rum-core' -import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' +import { LifeCycleEventType, SessionReplayState } from '@flashcatcloud/browser-rum-core' import { record } from '../domain/record' import { startSegmentCollection, SEGMENT_BYTES_LIMIT } from '../domain/segmentCollection' @@ -28,6 +28,10 @@ export function startRecording( let addRecord: (record: BrowserRecord) => void + // Assigned once recording has started. Segment collection is created first because `record()` + // emits into it, so the buffer reaches for the snapshot through this holder rather than directly. + let takeSubsequentFullSnapshot: () => void = noop + if (!canUseEventBridge()) { const segmentCollection = startSegmentCollection( lifeCycle, @@ -35,7 +39,18 @@ export function startRecording( sessionManager, viewHistory, replayRequest, - encoder + encoder, + { + getWithholdingSessionId: () => { + const session = sessionManager.findTrackedSession() + return session?.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR ? session.id : undefined + }, + isReleased: (sessionId) => { + const session = sessionManager.findTrackedSession() + return !!session && session.id === sessionId && session.sessionReplay !== SessionReplayState.BUFFERED_ON_ERROR + }, + restartFromFullSnapshot: () => takeSubsequentFullSnapshot(), + } ) addRecord = segmentCollection.addRecord cleanupTasks.push(segmentCollection.stop) @@ -43,13 +58,14 @@ export function startRecording( ;({ addRecord } = startRecordBridge(viewHistory)) } - const { stop: stopRecording } = record({ + const recording = record({ emit: addRecord, configuration, lifeCycle, viewHistory, }) - cleanupTasks.push(stopRecording) + takeSubsequentFullSnapshot = recording.takeSubsequentFullSnapshot + cleanupTasks.push(recording.stop) return { stop: () => { diff --git a/packages/rum/src/domain/getSessionReplayLink.ts b/packages/rum/src/domain/getSessionReplayLink.ts index 1bb7c38ea5..e8df168276 100644 --- a/packages/rum/src/domain/getSessionReplayLink.ts +++ b/packages/rum/src/domain/getSessionReplayLink.ts @@ -34,6 +34,11 @@ function getErrorType(session: RumSession | undefined, isRecordingStarted: boole // - replay sampled out return 'incorrect-session-plan' } + if (session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR) { + // the session records, but nothing has been uploaded yet and nothing may ever be: there is no + // replay to link to until the session reports an error + return 'replay-not-started' + } if (!isRecordingStarted) { return 'replay-not-started' } diff --git a/packages/rum/src/domain/record/record.ts b/packages/rum/src/domain/record/record.ts index 82e41cb1d6..c7187f1a44 100644 --- a/packages/rum/src/domain/record/record.ts +++ b/packages/rum/src/domain/record/record.ts @@ -33,6 +33,11 @@ export interface RecordOptions { export interface RecordAPI { stop: () => void flushMutations: () => void + /** + * Re-serializes the document so that the records that follow are replayable on their own. Needed + * when a withheld replay buffer is dropped, since it takes its full snapshot with it. + */ + takeSubsequentFullSnapshot: () => void shadowRootsController: ShadowRootsController } @@ -54,7 +59,7 @@ export function record(options: RecordOptions): RecordAPI { const shadowRootsController = initShadowRootsController(configuration, emitAndComputeStats, elementsScrollPositions) - const { stop: stopFullSnapshots } = startFullSnapshots( + const { stop: stopFullSnapshots, takeSubsequentFullSnapshot } = startFullSnapshots( elementsScrollPositions, shadowRootsController, lifeCycle, @@ -95,6 +100,7 @@ export function record(options: RecordOptions): RecordAPI { stopFullSnapshots() }, flushMutations, + takeSubsequentFullSnapshot, shadowRootsController, } } diff --git a/packages/rum/src/domain/record/startFullSnapshots.ts b/packages/rum/src/domain/record/startFullSnapshots.ts index 885d4ce31e..2438cd03a8 100644 --- a/packages/rum/src/domain/record/startFullSnapshots.ts +++ b/packages/rum/src/domain/record/startFullSnapshots.ts @@ -80,5 +80,19 @@ export function startFullSnapshots( return { stop: unsubscribe, + /** + * Re-serializes the document so that what follows is replayable on its own. Used when a withheld + * replay buffer is dropped: the records kept afterwards need a full snapshot to start from. + */ + takeSubsequentFullSnapshot: () => { + flushMutations() + fullSnapshotCallback( + takeFullSnapshot(timeStampNow(), { + shadowRootsController, + status: SerializationContextStatus.SUBSEQUENT_FULL_SNAPSHOT, + elementsScrollPositions, + }) + ) + }, } } diff --git a/packages/rum/src/domain/replayStats.ts b/packages/rum/src/domain/replayStats.ts index 76c5273f6c..a8945ff233 100644 --- a/packages/rum/src/domain/replayStats.ts +++ b/packages/rum/src/domain/replayStats.ts @@ -19,6 +19,21 @@ export function addWroteData(viewId: string, additionalBytesCount: number) { getOrCreateReplayStats(viewId).segments_total_raw_size += additionalBytesCount } +/** + * Rolls back what a segment contributed to the stats. Used when a withheld segment is dropped + * instead of sent: it never reached the intake, so it must leave no trace in the numbers reported + * on view events, and the next segment must reuse its `index_in_view`. + */ +export function discardSegment(viewId: string, rawBytesCount: number, recordsCount: number) { + const replayStats = statsPerView?.get(viewId) + if (!replayStats) { + return + } + replayStats.segments_count = Math.max(0, replayStats.segments_count - 1) + replayStats.records_count = Math.max(0, replayStats.records_count - recordsCount) + replayStats.segments_total_raw_size = Math.max(0, replayStats.segments_total_raw_size - rawBytesCount) +} + export function getReplayStats(viewId: string) { return statsPerView?.get(viewId) } diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index ad68e05fd8..8d65e1c37c 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -9,7 +9,9 @@ import type { BrowserRecord, SegmentContext } from '../../types' import { RecordType } from '../../types' import { MockWorker, readMetadataFromReplayPayload } from '../../../test' import { createDeflateEncoder } from '../deflate' +import * as replayStats from '../replayStats' import { + BUFFER_CHECKOUT_TIME, computeSegmentContext, doStartSegmentCollection, SEGMENT_BYTES_LIMIT, @@ -312,3 +314,231 @@ describe('computeSegmentContext', () => { } as any } }) + +describe('startSegmentCollection withholding (error session replay)', () => { + let clock: Clock + let lifeCycle: LifeCycle + let worker: MockWorker + let httpRequestSpy: { + sendOnExit: jasmine.Spy + send: jasmine.Spy + } + let addRecord: (record: BrowserRecord) => void + let withholdingSessionId: string | undefined + let releasedSessionId: string | undefined + let restartFromFullSnapshotSpy: jasmine.Spy<() => void> + + function reportError() { + releasedSessionId = withholdingSessionId + withholdingSessionId = undefined + } + + beforeEach(() => { + clock = mockClock() + lifeCycle = new LifeCycle() + worker = new MockWorker() + httpRequestSpy = { sendOnExit: jasmine.createSpy(), send: jasmine.createSpy() } + withholdingSessionId = CONTEXT.session.id + releasedSessionId = undefined + restartFromFullSnapshotSpy = jasmine.createSpy() + replayStats.resetReplayStats() + + const { stop, addRecord: add } = doStartSegmentCollection( + lifeCycle, + () => CONTEXT, + httpRequestSpy, + createDeflateEncoder({} as RumConfiguration, worker, DeflateEncoderStreamId.REPLAY), + { + getWithholdingSessionId: () => withholdingSessionId, + isReleased: (sessionId) => releasedSessionId === sessionId, + restartFromFullSnapshot: restartFromFullSnapshotSpy, + } + ) + addRecord = add + + registerCleanupTask(() => { + stop() + clock.cleanup() + replayStats.resetReplayStats() + }) + }) + + it('does not send anything while the session has not reported an error', () => { + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('keeps buffering across several duration limits instead of cutting the segment', () => { + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT * 3) + addRecord(RECORD) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + // still the same buffer: dropping it would have asked for a fresh full snapshot + expect(restartFromFullSnapshotSpy).not.toHaveBeenCalled() + }) + + it('sends the withheld buffer once the session reports an error', async () => { + addRecord(RECORD) + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + expect(httpRequestSpy.send).not.toHaveBeenCalled() + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + // the records collected before the error are part of what is sent + expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).records_count).toBe(2) + }) + + it('drops the buffer and restarts from a full snapshot once it spans the checkout time', () => { + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + }) + + it('drops the buffer and restarts from a full snapshot when it grows past the bytes limit', () => { + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + }) + + it('does not restart in a hot loop when the full snapshot alone exceeds the bytes limit', () => { + // every restart would blow the limit again straight away on such a document + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + + clock.tick(SEGMENT_DURATION_LIMIT) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(2) + }) + + it('drops the buffer on page exit rather than sending a replay for a session that never errored', () => { + addRecord(RECORD) + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + worker.processAllMessages() + + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('leaves no trace of a dropped buffer in the replay stats', () => { + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + worker.processAllMessages() + + const stats = replayStats.getReplayStats(CONTEXT.view.id) + expect(stats?.segments_count ?? 0).toBe(0) + expect(stats?.segments_total_raw_size ?? 0).toBe(0) + }) + + it('sends normally once released, without withholding the following segments', () => { + reportError() + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(2) + }) +}) + +describe('startSegmentCollection withholding, session lifecycle', () => { + let clock: Clock + let lifeCycle: LifeCycle + let worker: MockWorker + let httpRequestSpy: { + sendOnExit: jasmine.Spy + send: jasmine.Spy + } + let addRecord: (record: BrowserRecord) => void + let stopSegmentCollection: () => void + let withholdingSessionId: string | undefined + let releasedSessionId: string | undefined + + beforeEach(() => { + clock = mockClock() + lifeCycle = new LifeCycle() + worker = new MockWorker() + httpRequestSpy = { sendOnExit: jasmine.createSpy(), send: jasmine.createSpy() } + withholdingSessionId = CONTEXT.session.id + releasedSessionId = undefined + + const { stop, addRecord: add } = doStartSegmentCollection( + lifeCycle, + () => CONTEXT, + httpRequestSpy, + createDeflateEncoder({} as RumConfiguration, worker, DeflateEncoderStreamId.REPLAY), + { + getWithholdingSessionId: () => withholdingSessionId, + isReleased: (sessionId) => releasedSessionId === sessionId, + restartFromFullSnapshot: () => undefined, + } + ) + addRecord = add + stopSegmentCollection = stop + + registerCleanupTask(() => { + stopSegmentCollection() + clock.cleanup() + }) + }) + + it('drops the buffer when the session expires without ever reporting an error', () => { + addRecord(RECORD) + // the session is gone, so nothing answers for these records any more + withholdingSessionId = undefined + releasedSessionId = undefined + + stopSegmentCollection() + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('drops the buffer when the session is renewed into a different one', () => { + addRecord(RECORD) + withholdingSessionId = undefined + releasedSessionId = 'a-different-session' + + stopSegmentCollection() + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('sends the buffer when its own session reports the error', () => { + addRecord(RECORD) + releasedSessionId = withholdingSessionId + withholdingSessionId = undefined + + stopSegmentCollection() + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 259ca609d3..ad35a72b1a 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -1,13 +1,29 @@ -import type { DeflateEncoder, HttpRequest, TimeoutId } from '@flashcatcloud/browser-core' -import { isPageExitReason, ONE_SECOND, clearTimeout, setTimeout } from '@flashcatcloud/browser-core' +import type { DeflateEncoder, HttpRequest, RelativeTime, TimeoutId } from '@flashcatcloud/browser-core' +import { + addTelemetryDebug, + isPageExitReason, + ONE_SECOND, + clearTimeout, + noop, + relativeNow, + setTimeout, +} from '@flashcatcloud/browser-core' import type { LifeCycle, ViewHistory, RumSessionManager, RumConfiguration } from '@flashcatcloud/browser-rum-core' import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' +import { discardSegment } from '../replayStats' import { buildReplayPayload } from './buildReplayPayload' import type { FlushReason, Segment } from './segment' import { createSegment } from './segment' export const SEGMENT_DURATION_LIMIT = 5 * ONE_SECOND + +/** + * How much history a withheld buffer may span before it is dropped and restarted from a fresh full + * snapshot. This bounds two things at once: the memory a session that never errors holds on to, and + * how far back an error session can show once its buffer is released. + */ +export const BUFFER_CHECKOUT_TIME = 60 * ONE_SECOND /** * beacon payload max queue size implementation is 64kb * ensure that we leave room for logs, rum and potential other users @@ -39,19 +55,49 @@ export let SEGMENT_BYTES_LIMIT = 60_000 // To help investigate session replays issues, each segment is created with a "creation reason", // indicating why the session has been created. +/** + * Lets a session record without uploading anything until it reports an error. Sessions drawn by + * `sessionReplayOnErrorSampleRate` record from the start, but every segment is withheld: dropped on + * checkout while no error has happened, sent normally from the moment one has. + */ +export interface SegmentBuffering { + /** + * The id of the current session if it is withholding its replay, `undefined` otherwise. A segment + * remembers this at creation, so that what happens to it later is decided by the session that + * actually produced its records. + */ + getWithholdingSessionId: () => string | undefined + /** + * Whether that same session has since reported its error. Anything else — the session expired, or + * was renewed into a different one — means the records were never released and must be dropped: + * uploading them would bill a session for a replay nobody asked for and nobody can explain. + */ + isReleased: (sessionId: string) => boolean + /** Restarts the buffer from a fresh full snapshot, after the previous one was dropped. */ + restartFromFullSnapshot: () => void +} + +const NO_BUFFERING: SegmentBuffering = { + getWithholdingSessionId: () => undefined, + isReleased: () => false, + restartFromFullSnapshot: noop, +} + export function startSegmentCollection( lifeCycle: LifeCycle, configuration: RumConfiguration, sessionManager: RumSessionManager, viewHistory: ViewHistory, httpRequest: HttpRequest, - encoder: DeflateEncoder + encoder: DeflateEncoder, + buffering: SegmentBuffering = NO_BUFFERING ) { return doStartSegmentCollection( lifeCycle, () => computeSegmentContext(configuration.applicationId, sessionManager, viewHistory), httpRequest, - encoder + encoder, + buffering ) } @@ -69,22 +115,43 @@ type SegmentCollectionState = status: SegmentCollectionStatus.SegmentPending segment: Segment expirationTimeoutId: TimeoutId + /** Only armed while the segment is withheld: bounds how much history the buffer may span. */ + bufferCheckoutTimeoutId: TimeoutId | undefined + /** Set when the segment was created while its session was withholding its replay. */ + withheldForSessionId: string | undefined } | { status: SegmentCollectionStatus.Stopped } +/** + * `buffer_checkout` is internal: it drops a withheld buffer that has grown past + * {@link BUFFER_CHECKOUT_TIME}. It never reaches the intake, so it is mapped back to a schema value + * before being recorded as the next segment's creation reason. + */ +type InternalFlushReason = FlushReason | 'buffer_checkout' + +function toCreationReason(flushReason: Exclude): CreationReason { + return flushReason === 'buffer_checkout' ? 'segment_duration_limit' : flushReason +} + export function doStartSegmentCollection( lifeCycle: LifeCycle, getSegmentContext: () => SegmentContext | undefined, httpRequest: HttpRequest, - encoder: DeflateEncoder + encoder: DeflateEncoder, + buffering: SegmentBuffering = NO_BUFFERING ) { let state: SegmentCollectionState = { status: SegmentCollectionStatus.WaitingForInitialRecord, nextSegmentCreationReason: 'init', } + // How many buffers were dropped before one was finally released. Without this, "the replay goes + // back up to a minute" is a promise nobody can check. + let droppedBufferCount = 0 + let lastBufferRestartAt: RelativeTime | undefined + const { unsubscribe: unsubscribeViewCreated } = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, () => { flushSegment('view_change') }) @@ -96,9 +163,45 @@ export function doStartSegmentCollection( } ) - function flushSegment(flushReason: FlushReason) { + function flushSegment(flushReason: InternalFlushReason) { + // Decided once, and against the session that produced the records rather than whatever session + // is current now: a segment must be either dropped or sent as a whole. + const isWithheld = + state.status === SegmentCollectionStatus.SegmentPending && + state.withheldForSessionId !== undefined && + !buffering.isReleased(state.withheldForSessionId) + if (state.status === SegmentCollectionStatus.SegmentPending) { + if (isWithheld && flushReason === 'segment_duration_limit') { + // The 5s rotation is what turns records into requests. While withheld there is nothing to + // send, so the segment keeps growing instead, and the timer is re-armed so that the buffer + // is flushed normally within one rotation of the session reporting its error. + state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) + return + } + + const wasWithheld = state.withheldForSessionId !== undefined + state.segment.flush((metadata, encoderResult) => { + if (isWithheld) { + // No error was reported, so this buffer is dropped rather than sent. Rolling back its + // stats keeps `has_replay` and the replay counters reported on view events honest. + discardSegment(metadata.view.id, encoderResult.rawBytesCount, metadata.records_count) + droppedBufferCount += 1 + return + } + + if (wasWithheld) { + // The first segment released by an error: report how much history it actually carried, so + // the window we promise can be compared against the one users get. + addTelemetryDebug('Error session replay buffer released', { + 'buffer.duration': metadata.end - metadata.start, + 'buffer.records_count': metadata.records_count, + 'buffer.dropped_count': droppedBufferCount, + }) + droppedBufferCount = 0 + } + const payload = buildReplayPayload(encoderResult.output, metadata, encoderResult.rawBytesCount) if (isPageExitReason(flushReason)) { @@ -108,18 +211,32 @@ export function doStartSegmentCollection( } }) clearTimeout(state.expirationTimeoutId) + clearTimeout(state.bufferCheckoutTimeoutId) } if (flushReason !== 'stop') { state = { status: SegmentCollectionStatus.WaitingForInitialRecord, - nextSegmentCreationReason: flushReason, + nextSegmentCreationReason: toCreationReason(flushReason), } } else { state = { status: SegmentCollectionStatus.Stopped, } } + + // A dropped buffer leaves no full snapshot behind, so the next one would not be replayable on + // its own. A view change does not need this: the new view emits its own full snapshot. + if (isWithheld && (flushReason === 'buffer_checkout' || flushReason === 'segment_bytes_limit')) { + // On a document whose full snapshot alone exceeds the segment limit, every restart would blow + // the limit again straight away and restart once more. Spacing restarts out keeps that case at + // the cost of an ordinary segment rotation instead of a hot loop. + const now = relativeNow() + if (lastBufferRestartAt === undefined || now - lastBufferRestartAt >= SEGMENT_DURATION_LIMIT) { + lastBufferRestartAt = now + buffering.restartFromFullSnapshot() + } + } } return { @@ -134,12 +251,20 @@ export function doStartSegmentCollection( return } + const withheldForSessionId = buffering.getWithholdingSessionId() state = { status: SegmentCollectionStatus.SegmentPending, segment: createSegment({ encoder, context, creationReason: state.nextSegmentCreationReason }), expirationTimeoutId: setTimeout(() => { flushSegment('segment_duration_limit') }, SEGMENT_DURATION_LIMIT), + bufferCheckoutTimeoutId: + withheldForSessionId !== undefined + ? setTimeout(() => { + flushSegment('buffer_checkout') + }, BUFFER_CHECKOUT_TIME) + : undefined, + withheldForSessionId, } } From 5912e3e56793fa20ffe15a95540fa75e1fce718b Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 04:30:04 -0700 Subject: [PATCH 03/27] refactor(rum): name the session a withheld segment belongs to just once The flush path derived the same thing twice under two names, and the mapping of the internal checkout reason onto a schema value only ever had one caller. --- .../segmentCollection/segmentCollection.ts | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index ad35a72b1a..bd0e95a6b5 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -127,14 +127,10 @@ type SegmentCollectionState = /** * `buffer_checkout` is internal: it drops a withheld buffer that has grown past * {@link BUFFER_CHECKOUT_TIME}. It never reaches the intake, so it is mapped back to a schema value - * before being recorded as the next segment's creation reason. + * where the next segment records why it was created. */ type InternalFlushReason = FlushReason | 'buffer_checkout' -function toCreationReason(flushReason: Exclude): CreationReason { - return flushReason === 'buffer_checkout' ? 'segment_duration_limit' : flushReason -} - export function doStartSegmentCollection( lifeCycle: LifeCycle, getSegmentContext: () => SegmentContext | undefined, @@ -166,10 +162,9 @@ export function doStartSegmentCollection( function flushSegment(flushReason: InternalFlushReason) { // Decided once, and against the session that produced the records rather than whatever session // is current now: a segment must be either dropped or sent as a whole. - const isWithheld = - state.status === SegmentCollectionStatus.SegmentPending && - state.withheldForSessionId !== undefined && - !buffering.isReleased(state.withheldForSessionId) + const withheldForSessionId = + state.status === SegmentCollectionStatus.SegmentPending ? state.withheldForSessionId : undefined + const isWithheld = withheldForSessionId !== undefined && !buffering.isReleased(withheldForSessionId) if (state.status === SegmentCollectionStatus.SegmentPending) { if (isWithheld && flushReason === 'segment_duration_limit') { @@ -180,8 +175,6 @@ export function doStartSegmentCollection( return } - const wasWithheld = state.withheldForSessionId !== undefined - state.segment.flush((metadata, encoderResult) => { if (isWithheld) { // No error was reported, so this buffer is dropped rather than sent. Rolling back its @@ -191,7 +184,7 @@ export function doStartSegmentCollection( return } - if (wasWithheld) { + if (withheldForSessionId !== undefined) { // The first segment released by an error: report how much history it actually carried, so // the window we promise can be compared against the one users get. addTelemetryDebug('Error session replay buffer released', { @@ -217,7 +210,7 @@ export function doStartSegmentCollection( if (flushReason !== 'stop') { state = { status: SegmentCollectionStatus.WaitingForInitialRecord, - nextSegmentCreationReason: toCreationReason(flushReason), + nextSegmentCreationReason: flushReason === 'buffer_checkout' ? 'segment_duration_limit' : flushReason, } } else { state = { From 8ca8bca2f35e1a92abb956ded0ff10dd38459abb Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 04:50:55 -0700 Subject: [PATCH 04/27] fix(rum): keep a withheld replay buffer when the page is only hidden A page-exit rotation used to throw the buffer away, and with it the full snapshot a released replay has to start from - everything recorded afterwards is incremental and cannot be played on its own. Switching tabs raises this exit, and the page comes straight back, so an error reported after that would have released a replay that renders as good as nothing until the next view. Nothing can be sent while withheld, so there was never anything to gain from the rotation. A page that is really unloading takes the buffer with it either way. --- .../segmentCollection.spec.ts | 19 ++++++++++++++++++- .../segmentCollection/segmentCollection.ts | 15 ++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index 8d65e1c37c..9705fda634 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -434,7 +434,24 @@ describe('startSegmentCollection withholding (error session replay)', () => { expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(2) }) - it('drops the buffer on page exit rather than sending a replay for a session that never errored', () => { + it('keeps the buffer when the page is only hidden, so the replay can still start from its snapshot', () => { + // switching tabs is ordinary; dropping here would take the only full snapshot with it + addRecord(RECORD) + addRecord(RECORD) + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.HIDDEN }) + worker.processAllMessages() + + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).not.toHaveBeenCalled() + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + }) + + it('sends nothing on page exit for a session that never errored', () => { addRecord(RECORD) lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) worker.processAllMessages() diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index bd0e95a6b5..f999a2328e 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -167,11 +167,16 @@ export function doStartSegmentCollection( const isWithheld = withheldForSessionId !== undefined && !buffering.isReleased(withheldForSessionId) if (state.status === SegmentCollectionStatus.SegmentPending) { - if (isWithheld && flushReason === 'segment_duration_limit') { - // The 5s rotation is what turns records into requests. While withheld there is nothing to - // send, so the segment keeps growing instead, and the timer is re-armed so that the buffer - // is flushed normally within one rotation of the session reporting its error. - state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) + if (isWithheld && (flushReason === 'segment_duration_limit' || isPageExitReason(flushReason))) { + // Nothing can be sent while withheld, so these rotations would only throw the buffer away - + // and with it the full snapshot a released replay has to start from, leaving the rest of the + // session as incremental records nothing can be played from. A page that is merely hidden or + // frozen comes back and goes on recording; one that is really unloading takes the buffer with + // it either way. Keeping it is never worse than dropping it. + if (flushReason === 'segment_duration_limit') { + // Re-armed, so the buffer is flushed normally within one rotation of the session erroring. + state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) + } return } From 2c2c4f5a57876dcf01800233b29440d550cb0c18 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 06:52:41 -0700 Subject: [PATCH 05/27] docs(rum): record the window in which a session can take its own released buffer Only the rotation notices that the withheld replay has been released, so a session that expires within one rotation of its own error still loses what the error had earned. Closing it would mean asking the session manager on every record. --- .../rum/src/domain/segmentCollection/segmentCollection.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index f999a2328e..d74d02e2f5 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -175,6 +175,10 @@ export function doStartSegmentCollection( // it either way. Keeping it is never worse than dropping it. if (flushReason === 'segment_duration_limit') { // Re-armed, so the buffer is flushed normally within one rotation of the session erroring. + // That rotation is also the only thing that notices the release, which leaves a window of + // one rotation in which a session that expires right after its own error takes the buffer + // with it. Closing it would mean asking the session manager on every record, which is far + // too hot a path for a window this narrow. state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) } return From f516c9e34f908186703d80c001a8ce9115e02c7f Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 20:40:59 -0700 Subject: [PATCH 06/27] fix(rum): stop a dropped buffer leaving its segment index behind The rollback that gives a dropped buffer's index_in_view back only lands when the encoder finishes, which is always a turn later. Restarting from a fresh full snapshot emitted records right away, so the next segment took its index before the rollback arrived - and once that session errored, two uploaded segments claimed the same index within one view while nothing claimed the first. Any error session that spends a minute on one view before erroring hit it. The restart now happens where the rollback lands. Also corrects a comment: a session expiring right after its own error does not lose the buffer. The history entry is still open when the recorder is stopped, so the stop flush sees the session as released and sends. --- .../segmentCollection.spec.ts | 17 ++++++++ .../segmentCollection/segmentCollection.ts | 39 ++++++++++++------- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index 9705fda634..10170ddc82 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -459,6 +459,23 @@ describe('startSegmentCollection withholding (error session replay)', () => { expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() }) + it('does not let a dropped buffer leave its index_in_view behind for the next one to collide with', async () => { + // the restart emits records, exactly as taking a fresh full snapshot does in production + restartFromFullSnapshotSpy.and.callFake(() => addRecord(RECORD)) + + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + worker.processAllMessages() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + // the dropped buffer never reached the intake, so the first segment that does is index 0 + expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).index_in_view).toBe(0) + }) + it('leaves no trace of a dropped buffer in the replay stats', () => { addRecord(RECORD) clock.tick(BUFFER_CHECKOUT_TIME) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index d74d02e2f5..f42b7829bb 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -175,10 +175,9 @@ export function doStartSegmentCollection( // it either way. Keeping it is never worse than dropping it. if (flushReason === 'segment_duration_limit') { // Re-armed, so the buffer is flushed normally within one rotation of the session erroring. - // That rotation is also the only thing that notices the release, which leaves a window of - // one rotation in which a session that expires right after its own error takes the buffer - // with it. Closing it would mean asking the session manager on every record, which is far - // too hot a path for a window this narrow. + // An expiring session does not lose it: the session history entry is still open when the + // recorder is stopped (`sessionManager.ts` notifies before closing it), so the stop flush + // still sees the session as released and sends. Only losing the page outright loses it. state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) } return @@ -190,6 +189,10 @@ export function doStartSegmentCollection( // stats keeps `has_replay` and the replay counters reported on view events honest. discardSegment(metadata.view.id, encoderResult.rawBytesCount, metadata.records_count) droppedBufferCount += 1 + // Restarted from here rather than synchronously below: this callback is where the rollback + // lands, and a segment created before it would take an `index_in_view` this one still + // occupies - two uploaded segments would end up claiming the same index. + restartBuffer(flushReason) return } @@ -226,18 +229,24 @@ export function doStartSegmentCollection( status: SegmentCollectionStatus.Stopped, } } + } - // A dropped buffer leaves no full snapshot behind, so the next one would not be replayable on - // its own. A view change does not need this: the new view emits its own full snapshot. - if (isWithheld && (flushReason === 'buffer_checkout' || flushReason === 'segment_bytes_limit')) { - // On a document whose full snapshot alone exceeds the segment limit, every restart would blow - // the limit again straight away and restart once more. Spacing restarts out keeps that case at - // the cost of an ordinary segment rotation instead of a hot loop. - const now = relativeNow() - if (lastBufferRestartAt === undefined || now - lastBufferRestartAt >= SEGMENT_DURATION_LIMIT) { - lastBufferRestartAt = now - buffering.restartFromFullSnapshot() - } + /** + * A dropped buffer leaves no full snapshot behind, so the next one would not be replayable on its + * own. A view change does not need this: the new view emits its own full snapshot. + */ + function restartBuffer(flushReason: InternalFlushReason) { + if (flushReason !== 'buffer_checkout' && flushReason !== 'segment_bytes_limit') { + return + } + // On a document whose full snapshot alone exceeds the segment limit, every restart would blow + // the limit again straight away and restart once more. Spacing restarts out avoids that hot + // loop, at the cost of a buffer that carries no full snapshot until the next restart is allowed + // - if the error lands in that window, what is released cannot be played from its start. + const now = relativeNow() + if (lastBufferRestartAt === undefined || now - lastBufferRestartAt >= SEGMENT_DURATION_LIMIT) { + lastBufferRestartAt = now + buffering.restartFromFullSnapshot() } } From 32ade09152446fd03a41f1d4af6e7ca7e8fe75af Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 21 Aug 2026 03:00:28 -0700 Subject: [PATCH 07/27] feat(rum): mark a replay that is only kept because the session errored Without it, a replay collected under this rate is indistinguishable from one collected unconditionally once it has been uploaded - the two cost differently and answer different questions, and nothing downstream could tell them apart. --- developer-extension/package.json | 2 +- packages/core/package.json | 2 +- packages/flagging/package.json | 2 +- packages/logs/package.json | 4 ++-- packages/rum-core/package.json | 2 +- .../src/domain/contexts/sessionContext.ts | 5 +++++ .../src/domain/rumSessionManager.spec.ts | 21 +++++++++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 16 ++++++++++++-- .../rum-core/test/mockRumSessionManager.ts | 8 ++++++- packages/rum-react/package.json | 2 +- packages/rum-slim/package.json | 2 +- packages/rum/package.json | 2 +- packages/worker/package.json | 2 +- performances/package.json | 2 +- 14 files changed, 58 insertions(+), 14 deletions(-) diff --git a/developer-extension/package.json b/developer-extension/package.json index 4dfd2d82d8..9b3b33c883 100644 --- a/developer-extension/package.json +++ b/developer-extension/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-sdk-developer-extension", - "version": "0.0.2", + "version": "0.1.0", "private": true, "scripts": { "build": "rm -rf dist && webpack --mode production", diff --git a/packages/core/package.json b/packages/core/package.json index 5f10594575..0f1dbb5578 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-core", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/packages/flagging/package.json b/packages/flagging/package.json index 2358defd56..51fa4899c6 100644 --- a/packages/flagging/package.json +++ b/packages/flagging/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-flagging", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "private": true, "main": "cjs/entries/main.js", diff --git a/packages/logs/package.json b/packages/logs/package.json index 5de7c65469..fb74d47565 100644 --- a/packages/logs/package.json +++ b/packages/logs/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-logs", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", @@ -14,7 +14,7 @@ "replace-build-env": "node ../../scripts/build/replace-build-env.js" }, "dependencies": { - "@flashcatcloud/browser-core": "0.0.2" + "@flashcatcloud/browser-core": "0.1.0" }, "peerDependencies": { "@flashcatcloud/browser-rum": "0.0.2" diff --git a/packages/rum-core/package.json b/packages/rum-core/package.json index ea90c36a0d..b9ef0da2c1 100644 --- a/packages/rum-core/package.json +++ b/packages/rum-core/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-core", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index c8d893c110..a520a8524b 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -26,10 +26,14 @@ export function startSessionContext( let hasReplay let sampledForReplay + let sampledForErrorReplay let isActive if (eventType === RumEventType.VIEW) { hasReplay = !isReplayWithheld && recorderApi.getReplayStats(view.id) ? true : undefined sampledForReplay = session.sessionReplay === SessionReplayState.SAMPLED + // Tells a replay collected only because the session errored apart from one collected + // unconditionally - the two cost differently and are answered by different questions. + sampledForErrorReplay = session.sampledOnErrorReplay || undefined isActive = view.sessionIsActive ? undefined : false } else { hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined @@ -42,6 +46,7 @@ export function startSessionContext( type: SessionType.USER, has_replay: hasReplay, sampled_for_replay: sampledForReplay, + sampled_for_error_replay: sampledForErrorReplay, is_active: isActive, }, } diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 0c286da966..21dd0a499d 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -248,6 +248,27 @@ describe('rum session manager', () => { expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) }) + it('marks the session so a replay kept only because it errored can be told apart', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() + + // still true once released, so what was stored can be told apart afterwards + sessionManager.setSessionHasError() + + expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() + }) + + it('does not mark a session whose replay is collected unconditionally', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100 }, + }) + + expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeFalse() + }) + it('releases the replay when it is forced, rather than waiting for an error that may never come', () => { const sessionManager = startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index e58383718a..02ddb45388 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -34,6 +34,12 @@ export interface RumSessionManager { export type RumSession = { id: string sessionReplay: SessionReplayState + /** + * Whether the replay of this session is only kept if it reports an error. Unlike + * {@link sessionReplay} this stays true once the error has been reported, so a replay collected + * that way can be told apart from one collected unconditionally. + */ + sampledOnErrorReplay: boolean anonymousId?: string } @@ -99,6 +105,7 @@ export function startRumSessionManager( return { id: session.id, sessionReplay: computeSessionReplayState(session.trackingType, session.hasError, session.isReplayForced), + sampledOnErrorReplay: withholdsReplay(session.trackingType), anonymousId: session.anonymousId, } }, @@ -109,6 +116,10 @@ export function startRumSessionManager( } } +export function withholdsReplay(trackingType: RumTrackingType) { + return trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY +} + export function computeSessionReplayState( trackingType: RumTrackingType, hasError: boolean, @@ -117,7 +128,7 @@ export function computeSessionReplayState( if (trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY) { return SessionReplayState.SAMPLED } - if (trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY && hasError) { + if (withholdsReplay(trackingType) && hasError) { return SessionReplayState.SAMPLED } // A forced replay wins over withholding: the host explicitly asked for this user's replay, so it @@ -125,7 +136,7 @@ export function computeSessionReplayState( if (isReplayForced) { return SessionReplayState.FORCED } - if (trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY) { + if (withholdsReplay(trackingType)) { return SessionReplayState.BUFFERED_ON_ERROR } return SessionReplayState.OFF @@ -138,6 +149,7 @@ export function startRumSessionManagerStub(): RumSessionManager { const session: RumSession = { id: '00000000-aaaa-0000-aaaa-000000000000', sessionReplay: bridgeSupports(BridgeCapability.RECORDS) ? SessionReplayState.SAMPLED : SessionReplayState.OFF, + sampledOnErrorReplay: false, } return { findTrackedSession: () => session, diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 9314b732a9..a97a96fba0 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -1,5 +1,10 @@ import { Observable } from '@flashcatcloud/browser-core' -import { RumTrackingType, computeSessionReplayState, type RumSessionManager } from '../src/domain/rumSessionManager' +import { + RumTrackingType, + computeSessionReplayState, + withholdsReplay, + type RumSessionManager, +} from '../src/domain/rumSessionManager' export interface RumSessionManagerMock extends RumSessionManager { setId(id: string): RumSessionManagerMock @@ -41,6 +46,7 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { id, // Derived the same way as in production, so the mock cannot drift from the real state machine sessionReplay: computeSessionReplayState(trackingType, hasError, forcedReplay), + sampledOnErrorReplay: withholdsReplay(trackingType), anonymousId: 'device-123', } }, diff --git a/packages/rum-react/package.json b/packages/rum-react/package.json index 0ab2acaf73..55c4336a26 100644 --- a/packages/rum-react/package.json +++ b/packages/rum-react/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-react", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/rum-slim/package.json b/packages/rum-slim/package.json index 01a559216e..a83bbb4873 100644 --- a/packages/rum-slim/package.json +++ b/packages/rum-slim/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-slim", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/rum/package.json b/packages/rum/package.json index 2e6bc64d23..c2bad47a7e 100644 --- a/packages/rum/package.json +++ b/packages/rum/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/worker/package.json b/packages/worker/package.json index 266fe2e8f6..b20c207129 100644 --- a/packages/worker/package.json +++ b/packages/worker/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-worker", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "bundle/worker.js", "scripts": { diff --git a/performances/package.json b/performances/package.json index 02eca5dbe0..780dddd734 100644 --- a/performances/package.json +++ b/performances/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "performances", - "version": "0.0.2", + "version": "0.1.0", "scripts": { "start": "ts-node ./src/main.ts" }, From 9cf3404485ccc8f57b74eb8fbea8949ebf7d1041 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 08:52:40 -0700 Subject: [PATCH 08/27] fix(rum): leave sessions that withhold nothing out of the session store Marking a session as having reported an error is only useful to a session that is withholding its replay. Doing it for every session wrote the session store for customers who enabled no error sampling at all, and that write also pushes the session's expiry out, which moves where their sessions end. --- .../src/domain/trackSessionError.spec.ts | 18 +++++++++++++++++- .../rum-core/src/domain/trackSessionError.ts | 8 ++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts index 696413d97e..05b254496c 100644 --- a/packages/rum-core/src/domain/trackSessionError.spec.ts +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -17,7 +17,7 @@ describe('startSessionErrorTracking', () => { beforeEach(() => { lifeCycle = new LifeCycle() - sessionManager = createRumSessionManagerMock() + sessionManager = createRumSessionManagerMock().setTrackedWithErrorSessionReplay() setSessionHasErrorSpy = spyOn(sessionManager, 'setSessionHasError').and.callThrough() const { stop } = startSessionErrorTracking(lifeCycle, sessionManager) registerCleanupTask(stop) @@ -29,6 +29,22 @@ describe('startSessionErrorTracking', () => { expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) }) + it('leaves a session that withholds nothing alone, so an ordinary session store is never written', () => { + sessionManager.setTrackedWithSessionReplay() + + collect('error') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('leaves an untracked session alone', () => { + sessionManager.setNotTracked() + + collect('error') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + it('does not mark the session on other event types', () => { collect('view') collect('resource') diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts index e4b54fb058..8946faf2af 100644 --- a/packages/rum-core/src/domain/trackSessionError.ts +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -25,6 +25,14 @@ export function startSessionErrorTracking(lifeCycle: LifeCycle, sessionManager: if (event.error.source === ErrorSource.AGENT) { return } + // Only a session that is withholding something has any use for this mark. Setting it on any + // other session would write the session store for customers who enabled neither rate - and that + // write also pushes the session's expiry out (`processSessionStoreOperations` expands every + // state it persists), which would move where their sessions end. + const session = sessionManager.findTrackedSession() + if (!session?.sampledOnErrorReplay) { + return + } hasReportedError = true sessionManager.setSessionHasError() }) From a85a57e3114a526eb8a58d6d87651bc9eaf7557c Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 08:52:40 -0700 Subject: [PATCH 09/27] fix(rum): give a dropped segment's index back before another one can take it Flushing a segment always waits for a round trip to the deflate worker, because the trailer is written just before finishing. The collection state is reset synchronously, so a record arriving during that round trip created the next segment while the dropped one was still counted: two uploaded segments then claimed the same index_in_view, and index 0 was never uploaded at all. Each counter is now given back in the phase it was taken in - the segment count synchronously, the record and byte counts in the flush callback. --- packages/rum/src/domain/replayStats.ts | 20 ++++++++++++---- .../segmentCollection.spec.ts | 18 ++++++++++++++ .../segmentCollection/segmentCollection.ts | 24 +++++++++++++------ 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/packages/rum/src/domain/replayStats.ts b/packages/rum/src/domain/replayStats.ts index a8945ff233..3a69ce57c1 100644 --- a/packages/rum/src/domain/replayStats.ts +++ b/packages/rum/src/domain/replayStats.ts @@ -20,16 +20,28 @@ export function addWroteData(viewId: string, additionalBytesCount: number) { } /** - * Rolls back what a segment contributed to the stats. Used when a withheld segment is dropped - * instead of sent: it never reached the intake, so it must leave no trace in the numbers reported - * on view events, and the next segment must reuse its `index_in_view`. + * Gives back the segment count {@link addSegment} took, and with it the `index_in_view` the segment + * was holding. Undone in the same phase it was taken - synchronously - because the index is read at + * creation: a segment created before this runs would hold an index the dropped one still occupies. */ -export function discardSegment(viewId: string, rawBytesCount: number, recordsCount: number) { +export function removeSegment(viewId: string) { const replayStats = statsPerView?.get(viewId) if (!replayStats) { return } replayStats.segments_count = Math.max(0, replayStats.segments_count - 1) +} + +/** + * Rolls back what a dropped segment's records contributed. These are the counters reported on view + * events, and a withheld segment that is dropped never reached the intake, so it must leave no + * trace in them. + */ +export function discardSegmentData(viewId: string, rawBytesCount: number, recordsCount: number) { + const replayStats = statsPerView?.get(viewId) + if (!replayStats) { + return + } replayStats.records_count = Math.max(0, replayStats.records_count - recordsCount) replayStats.segments_total_raw_size = Math.max(0, replayStats.segments_total_raw_size - rawBytesCount) } diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index 10170ddc82..fa70f6788d 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -476,6 +476,24 @@ describe('startSegmentCollection withholding (error session replay)', () => { expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).index_in_view).toBe(0) }) + it('does not hand the next segment an index the dropped one still holds when a record lands mid-flush', async () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(RECORD)) + + addRecord(RECORD) + // The flush is posted to the worker but not answered yet - in production that round trip always + // happens, because flushing writes the trailer before finishing. A record arriving now creates + // the next segment, which reads its index while the dropped one is still counted. + clock.tick(BUFFER_CHECKOUT_TIME) + addRecord(RECORD) + worker.processAllMessages() + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).index_in_view).toBe(0) + }) + it('leaves no trace of a dropped buffer in the replay stats', () => { addRecord(RECORD) clock.tick(BUFFER_CHECKOUT_TIME) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index f42b7829bb..4d64b84e83 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -11,7 +11,7 @@ import { import type { LifeCycle, ViewHistory, RumSessionManager, RumConfiguration } from '@flashcatcloud/browser-rum-core' import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' -import { discardSegment } from '../replayStats' +import { discardSegmentData, removeSegment } from '../replayStats' import { buildReplayPayload } from './buildReplayPayload' import type { FlushReason, Segment } from './segment' import { createSegment } from './segment' @@ -119,6 +119,8 @@ type SegmentCollectionState = bufferCheckoutTimeoutId: TimeoutId | undefined /** Set when the segment was created while its session was withholding its replay. */ withheldForSessionId: string | undefined + /** The view the segment belongs to, so its index can be given back without waiting on a flush. */ + viewId: string } | { status: SegmentCollectionStatus.Stopped @@ -183,15 +185,22 @@ export function doStartSegmentCollection( return } + if (isWithheld) { + // Given back here, synchronously, rather than in the flush callback below: that callback only + // runs after a round trip to the deflate worker, and a record arriving in between creates a + // segment that reads its `index_in_view` from a count this one still occupies - leaving two + // uploaded segments claiming the same index, and index 0 never uploaded at all. + removeSegment(state.viewId) + } + state.segment.flush((metadata, encoderResult) => { if (isWithheld) { - // No error was reported, so this buffer is dropped rather than sent. Rolling back its - // stats keeps `has_replay` and the replay counters reported on view events honest. - discardSegment(metadata.view.id, encoderResult.rawBytesCount, metadata.records_count) + // No error was reported, so this buffer is dropped rather than sent. Rolling back what its + // records contributed keeps `has_replay` and the counters on view events honest. + discardSegmentData(metadata.view.id, encoderResult.rawBytesCount, metadata.records_count) droppedBufferCount += 1 - // Restarted from here rather than synchronously below: this callback is where the rollback - // lands, and a segment created before it would take an `index_in_view` this one still - // occupies - two uploaded segments would end up claiming the same index. + // Restarted from here rather than synchronously below, so the fresh full snapshot lands in + // the segment that follows this one rather than in the one being thrown away. restartBuffer(flushReason) return } @@ -276,6 +285,7 @@ export function doStartSegmentCollection( }, BUFFER_CHECKOUT_TIME) : undefined, withheldForSessionId, + viewId: context.view.id, } } From 38fe32b9f6da93958d970be69299c57921d1cf2a Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:15:02 -0700 Subject: [PATCH 10/27] refactor(core): let a session store update see the state it would land on A store write goes through a lock and can be retried for up to a second, and other tabs write the same store meanwhile - so the state a write lands on is not necessarily the one it was decided against. Updates are now expressed as a function of that state, and returning nothing makes the write a no-op, which is what a caller needs to say "only if this is still the session I meant". --- .../core/src/domain/session/sessionManager.spec.ts | 2 +- packages/core/src/domain/session/sessionManager.ts | 2 +- .../core/src/domain/session/sessionStore.spec.ts | 2 +- packages/core/src/domain/session/sessionStore.ts | 14 +++++++++++--- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index 345502f1d6..f3d188ab22 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -637,7 +637,7 @@ describe('startSessionManager', () => { const sessionManager = startSessionManagerWithDefaults() sessionManager.sessionStateUpdateObservable.subscribe(sessionStateUpdateSpy) - sessionManager.updateSessionState({ extra: 'extra' }) + sessionManager.updateSessionState(() => ({ extra: 'extra' })) expectSessionIdToBeDefined(sessionManager) expect(sessionStateUpdateSpy).toHaveBeenCalledTimes(1) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 789d0d5487..fae90339a5 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -20,7 +20,7 @@ export interface SessionManager { expireObservable: Observable sessionStateUpdateObservable: Observable<{ previousState: SessionState; newState: SessionState }> expire: () => void - updateSessionState: (state: Partial) => void + updateSessionState: (update: (state: SessionState) => Partial | undefined) => void } export interface SessionContext extends Context { diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 7d8105177c..2bcd9378f7 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -596,7 +596,7 @@ describe('session store', () => { sessionStoreManager = setupSessionStore(updateSpy) otherSessionStoreManager = setupSessionStore(otherUpdateSpy) - sessionStoreManager.updateSessionState({ extra: 'extra' }) + sessionStoreManager.updateSessionState(() => ({ extra: 'extra' })) expect(updateSpy).toHaveBeenCalledTimes(1) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 4c0a1a74e5..accc0b8184 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -28,7 +28,12 @@ export interface SessionStore { sessionStateUpdateObservable: Observable<{ previousState: SessionState; newState: SessionState }> expire: () => void stop: () => void - updateSessionState: (state: Partial) => void + /** + * Applies a change to the stored session under the store lock. The producer sees the state the + * change would land on and returns `undefined` to make it a no-op - which is how a write meant for + * one session avoids landing on the one that replaced it while the write was waiting for the lock. + */ + updateSessionState: (update: (state: SessionState) => Partial | undefined) => void } /** @@ -203,10 +208,13 @@ export function startSessionStore( renewObservable.notify() } - function updateSessionState(partialSessionState: Partial) { + function updateSessionState(update: (state: SessionState) => Partial | undefined) { processSessionStoreOperations( { - process: (sessionState) => ({ ...sessionState, ...partialSessionState }), + process: (sessionState) => { + const partialSessionState = update(sessionState) + return partialSessionState && { ...sessionState, ...partialSessionState } + }, after: synchronizeSession, }, sessionStoreStrategy From 4509cd491205928b78a6f7ee5cb4062b947dd585 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:15:02 -0700 Subject: [PATCH 11/27] fix(rum): keep the error mark on the session that reported the error Marking a session as having errored merged into whatever session the store held at the moment the write went through. A session that rolled over while the write waited for the lock - or that another tab renewed - was marked instead, and then uploaded a whole session that never reported anything. The mark now names the session it belongs to and is dropped if that session is gone. The same mark is also applied to the in-memory session straight away rather than only once the write lands, because until then the withheld buffer still reads the session as withholding: an error followed closely by the page or the session ending threw away the very buffer the error was meant to release. --- .../src/domain/rumSessionManager.spec.ts | 36 ++++++++++++++++++- .../rum-core/src/domain/rumSessionManager.ts | 21 ++++++++--- .../rum-core/src/domain/trackSessionError.ts | 2 +- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 21dd0a499d..438f188a6c 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -9,6 +9,7 @@ import { createTrackingConsentState, TrackingConsent, BridgeCapability, + isChromium, } from '@flashcatcloud/browser-core' import type { Clock } from '@flashcatcloud/browser-core/test' import { @@ -235,8 +236,41 @@ describe('rum session manager', () => { expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) - sessionManager.setSessionHasError() + sessionManager.setSessionHasError(sessionManager.findTrackedSession()!.id) + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('does not mark a session that has since been replaced by another one', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + // another tab renewed the session while the mark was on its way to the store + setCookie(SESSION_STORE_KEY, 'id=other-session&rum=3', DURATION) + + sessionManager.setSessionHasError('a-session-that-is-gone') + + expect(getSessionState(SESSION_STORE_KEY).hasError).toBeUndefined() + }) + + it('releases the replay before the store write lands, since that write can be deferred', () => { + if (!isChromium()) { + pending('the store lock, and so a deferred write, only exists on Chromium') + } + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + const sessionId = sessionManager.findTrackedSession()!.id + + // another tab holds the store lock, so the write is deferred through retries + setCookie(SESSION_STORE_KEY, `lock=other-tab&id=${sessionId}&rum=3`, DURATION) + + sessionManager.setSessionHasError(sessionId) + expect(getSessionState(SESSION_STORE_KEY).hasError).toBeUndefined() + // and yet the buffer must already see it as released: the page or the session may end before + // the write ever lands, and the buffer would otherwise be thrown away expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) }) diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 02ddb45388..ef6bcc4018 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -25,10 +25,11 @@ export interface RumSessionManager { expireObservable: Observable setForcedReplay: () => void /** - * Marks the session as having reported an error. For a session sampled by - * `sessionReplayOnErrorSampleRate`, this is what releases the withheld replay. + * Marks the given session as having reported an error. For a session sampled by + * `sessionReplayOnErrorSampleRate`, this is what releases the withheld replay. The id is required + * because the store write can be deferred by the lock, and it must not land on a later session. */ - setSessionHasError: () => void + setSessionHasError: (sessionId: string) => void } export type RumSession = { @@ -111,8 +112,18 @@ export function startRumSessionManager( }, expire: sessionManager.expire, expireObservable: sessionManager.expireObservable, - setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), - setSessionHasError: () => sessionManager.updateSessionState({ hasError: '1' }), + setForcedReplay: () => sessionManager.updateSessionState(() => ({ forcedReplay: '1' })), + setSessionHasError: (sessionId) => { + const sessionEntity = sessionManager.findSession() + if (sessionEntity?.id === sessionId) { + // Marked in memory straight away, and not only once the store write lands: that write goes + // through a lock that can defer it by several retries, and until then the withheld buffer + // would still read the session as withholding - so an error followed closely by the page or + // the session ending would throw away the very buffer the error was meant to release. + sessionEntity.hasError = true + } + sessionManager.updateSessionState((state) => (state.id === sessionId ? { hasError: '1' } : undefined)) + }, } } diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts index 8946faf2af..531d7f6aa5 100644 --- a/packages/rum-core/src/domain/trackSessionError.ts +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -34,7 +34,7 @@ export function startSessionErrorTracking(lifeCycle: LifeCycle, sessionManager: return } hasReportedError = true - sessionManager.setSessionHasError() + sessionManager.setSessionHasError(session.id) }) // A renewed session is a different session: it draws its own sampling and starts out without an From 3f3d27f9a3da628d2c8d1cd44b4b4c58ec200c5c Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:15:02 -0700 Subject: [PATCH 12/27] fix(rum): leave a stopped recorder alone when a late flush lands Dropping a withheld buffer restarts it from a fresh full snapshot, and that runs in a flush callback which only arrives after a round trip to the deflate worker. Recording stopped in between still got a full re-serialization of the document, and its records counted into the replay stats with no segment to hold them. --- .../segmentCollection/segmentCollection.spec.ts | 12 ++++++++++++ .../domain/segmentCollection/segmentCollection.ts | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index fa70f6788d..f765d45fd0 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -327,6 +327,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { let withholdingSessionId: string | undefined let releasedSessionId: string | undefined let restartFromFullSnapshotSpy: jasmine.Spy<() => void> + let stopCollection: () => void function reportError() { releasedSessionId = withholdingSessionId @@ -355,6 +356,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { } ) addRecord = add + stopCollection = stop registerCleanupTask(() => { stop() @@ -476,6 +478,16 @@ describe('startSegmentCollection withholding (error session replay)', () => { expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).index_in_view).toBe(0) }) + it('does not restart the buffer when collection was stopped while the flush was in flight', () => { + addRecord(RECORD) + // the checkout flush is posted to the worker, and recording is stopped before it answers + clock.tick(BUFFER_CHECKOUT_TIME) + stopCollection() + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).not.toHaveBeenCalled() + }) + it('does not hand the next segment an index the dropped one still holds when a record lands mid-flush', async () => { restartFromFullSnapshotSpy.and.callFake(() => addRecord(RECORD)) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 4d64b84e83..7953a09ffa 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -248,6 +248,12 @@ export function doStartSegmentCollection( if (flushReason !== 'buffer_checkout' && flushReason !== 'segment_bytes_limit') { return } + if (state.status === SegmentCollectionStatus.Stopped) { + // The flush that got here waited on the deflate worker, and recording was stopped in the + // meantime. Re-serializing the document now would cost a full snapshot on a page that asked + // to stop, and count records into the replay stats that no segment will ever hold. + return + } // On a document whose full snapshot alone exceeds the segment limit, every restart would blow // the limit again straight away and restart once more. Spacing restarts out avoids that hot // loop, at the cost of a buffer that carries no full snapshot until the next restart is allowed From b77b205275d68729720dc5e249c8bafcbaf715f3 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:34:06 -0700 Subject: [PATCH 13/27] fix(rum): send a withheld replay only when its own session earned it Whether a withheld replay had been released was inferred from the session no longer withholding. That is true for a reason other than an error: a session store is shared with every other SDK bundle on the domain, and one that predates these tracking types does not recognise them, so it redraws the session and rewrites the type. The withheld replay was then uploaded for a session that never reported anything. Release now requires the session to still be one whose replay is kept on an error. Any other transition ends the buffer instead of sending it. --- packages/rum/src/boot/startRecording.spec.ts | 21 ++++++++++++++++++++ packages/rum/src/boot/startRecording.ts | 11 +++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/rum/src/boot/startRecording.spec.ts b/packages/rum/src/boot/startRecording.spec.ts index 31f2b2bc84..e1527e0a9d 100644 --- a/packages/rum/src/boot/startRecording.spec.ts +++ b/packages/rum/src/boot/startRecording.spec.ts @@ -116,6 +116,27 @@ describe('startRecording', () => { expect(requests[0].metadata.records_count).toBe(1 + recordsPerFullSnapshot()) }) + it('drops a withheld replay when the session stops withholding without having errored', async () => { + sessionManager.setTrackedWithErrorSessionReplay() + setupStartRecording() + + document.body.dispatchEvent(createNewEvent('click', { clientX: 1, clientY: 2 })) + + // an older SDK sharing the same session store does not know this tracking type and redraws it. + // The session stops withholding, but it never reported an error, so what it held is not owed a + // trip to the intake. + sessionManager.setTrackedWithSessionReplay() + changeView(lifeCycle) + + document.body.dispatchEvent(createNewEvent('click', { clientX: 3, clientY: 4 })) + flushSegment(lifeCycle) + + const requests = await readSentRequests(1) + // 'init' would be the withheld segment; the first one to reach the intake is the one created + // after the session stopped withholding + expect(requests[0].metadata.creation_reason).toBe('view_change') + }) + it('restarts sending segments when the session is renewed', async () => { sessionManager.setNotTracked() setupStartRecording() diff --git a/packages/rum/src/boot/startRecording.ts b/packages/rum/src/boot/startRecording.ts index 1cc4aba3c3..8fc5cf3278 100644 --- a/packages/rum/src/boot/startRecording.ts +++ b/packages/rum/src/boot/startRecording.ts @@ -47,7 +47,16 @@ export function startRecording( }, isReleased: (sessionId) => { const session = sessionManager.findTrackedSession() - return !!session && session.id === sessionId && session.sessionReplay !== SessionReplayState.BUFFERED_ON_ERROR + // Still the same session, still one whose replay is kept only on an error, and no longer + // withholding. The middle condition matters: a session can stop withholding without ever + // erroring - an older SDK sharing the same store does not know these tracking types and + // redraws them - and that is a session ending, not a replay earning its way out. + return ( + !!session && + session.id === sessionId && + session.sampledOnErrorReplay && + session.sessionReplay !== SessionReplayState.BUFFERED_ON_ERROR + ) }, restartFromFullSnapshot: () => takeSubsequentFullSnapshot(), } From 082b9aa95b5f175976866792307b6824771c0a7a Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:52:35 -0700 Subject: [PATCH 14/27] fix(rum): do not offer a replay a dropped buffer took with it `has_replay` was set for any view that had replay stats at all. A withheld buffer that is dropped rolls its segments back, so a view can be left with stats and no replay - and the session then offers a replay nothing can play. It now takes a segment that survived. --- .../src/domain/contexts/sessionContext.spec.ts | 13 +++++++++++++ .../rum-core/src/domain/contexts/sessionContext.ts | 6 +++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index c8aec0c702..3d72ad5199 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -94,6 +94,19 @@ describe('session context', () => { expect(eventWithoutHasReplay.session!.has_replay).toBeUndefined() }) + it('should not set hasReplay when a dropped buffer left the view with no segment', () => { + // a withheld buffer that was dropped rolls its segments back, and a view left with zero of them + // has no replay to offer however many records were once counted + getReplayStatsSpy.and.returnValue({ ...fakeStats, segments_count: 0 }) + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.has_replay).toBeUndefined() + }) + it('should set session.is_active when the session is active', () => { findViewSpy.and.returnValue({ ...fakeView, sessionIsActive: true }) const eventWithActiveSession = hooks.triggerHook(HookNames.Assemble, { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index a520a8524b..289c33fa8e 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -29,7 +29,11 @@ export function startSessionContext( let sampledForErrorReplay let isActive if (eventType === RumEventType.VIEW) { - hasReplay = !isReplayWithheld && recorderApi.getReplayStats(view.id) ? true : undefined + // Counted rather than merely present: a withheld buffer that was dropped rolls its segments + // back, which leaves a view with replay stats and no replay at all - and offering a replay + // that was never uploaded is worse than not offering one. + const replayStats = recorderApi.getReplayStats(view.id) + hasReplay = !isReplayWithheld && replayStats && replayStats.segments_count > 0 ? true : undefined sampledForReplay = session.sessionReplay === SessionReplayState.SAMPLED // Tells a replay collected only because the session errored apart from one collected // unconditionally - the two cost differently and are answered by different questions. From ec332013719da37c40547a356265c9b5cd59adae Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:06:37 -0700 Subject: [PATCH 15/27] fix(rum): let forcing a replay reach a session that is withholding one Forcing a replay was only ever applied to a session whose replay was off, and only when the recorder was not already running. A session withholding its replay fails both: it is recording, and its replay is not off. So the session manager's rule that a forced replay wins over withholding, and releases the events with it, could not be reached from the public API at all - `startSessionReplayRecording({ force: true })` did nothing for exactly the sessions where it has something to do. --- packages/rum/src/boot/postStartStrategy.ts | 19 ++++++++++++++----- packages/rum/src/boot/recorderApi.spec.ts | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/rum/src/boot/postStartStrategy.ts b/packages/rum/src/boot/postStartStrategy.ts index c9678c6f21..58e1854666 100644 --- a/packages/rum/src/boot/postStartStrategy.ts +++ b/packages/rum/src/boot/postStartStrategy.ts @@ -87,6 +87,13 @@ export function createPostStartStrategy( return } + if (shouldForceReplay(session!, options)) { + // Applied before the guard below, not after starting: a session that withholds its replay is + // already recording, so the guard would return without ever releasing it - and releasing what + // is held is the whole of what forcing means for such a session. + sessionManager.setForcedReplay() + } + if (isRecordingInProgress(status)) { return } @@ -95,10 +102,6 @@ export function createPostStartStrategy( // Intentionally not awaiting doStart() to keep it asynchronous doStart().catch(monitorError) - - if (shouldForceReplay(session!, options)) { - sessionManager.setForcedReplay() - } } function stop() { @@ -128,5 +131,11 @@ function isRecordingInProgress(status: RecorderStatus) { } function shouldForceReplay(session: RumSession, options?: StartRecordingOptions) { - return options && options.force && session.sessionReplay === SessionReplayState.OFF + return ( + options && + options.force && + // A withheld replay is as much in need of forcing as one that was never sampled: the host asked + // for this user's replay, so it must not go on waiting for an error that may never come. + (session.sessionReplay === SessionReplayState.OFF || session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR) + ) } diff --git a/packages/rum/src/boot/recorderApi.spec.ts b/packages/rum/src/boot/recorderApi.spec.ts index b61ae8ea00..3c839095ff 100644 --- a/packages/rum/src/boot/recorderApi.spec.ts +++ b/packages/rum/src/boot/recorderApi.spec.ts @@ -178,6 +178,27 @@ describe('makeRecorderApi', () => { expect(setForcedReplaySpy).toHaveBeenCalledTimes(1) }) + it('releases a withheld replay when forced, although it is already recording', async () => { + const setForcedReplaySpy = jasmine.createSpy() + + setupRecorderApi({ + sessionManager: { + ...createRumSessionManagerMock().setTrackedWithErrorSessionReplay(), + setForcedReplay: setForcedReplaySpy, + }, + startSessionReplayRecordingManually: false, + }) + + rumInit() + await collectAsyncCalls(startRecordingSpy, 1) + + // the recording is already running - what forcing asks for here is that what it holds stops + // waiting for an error + recorderApi.start({ force: true }) + + expect(setForcedReplaySpy).toHaveBeenCalledTimes(1) + }) + it('uses the previously created worker if available', async () => { setupRecorderApi({ startSessionReplayRecordingManually: true }) rumInit({ worker: mockWorker }) From d1e2f3ee0425f883adb9a0217febd0ec7ba9996e Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:11:25 -0700 Subject: [PATCH 16/27] feat(rum): say so when a sampling rate cannot draw a single session sessionReplayOnErrorSampleRate is drawn from what the plain replay rate did not take, so some perfectly valid configurations can never draw anything: a plain rate of 100 leaves it nothing, a session rate of 0 leaves no session to draw from, and starting the recording manually leaves nothing recorded to withhold. Each of those now says so once at init. The option's own description also led with "the percentage of tracked sessions", which is not the base it is drawn from. --- .../configuration/configuration.spec.ts | 42 +++++++++++++++++++ .../src/domain/configuration/configuration.ts | 26 ++++++++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index a2c4c48875..6c39bb7cfa 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -65,6 +65,48 @@ describe('validateAndBuildRumConfiguration', () => { }) }) + describe('sessionReplayOnErrorSampleRate', () => { + it('warns when the plain replay rate leaves it nothing to draw from', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 100, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('warns when no session is tracked at all', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 0, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('warns when the recording is left for the customer to start, since nothing would be held', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplayOnErrorSampleRate: 50, + startSessionReplayRecordingManually: true, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('says nothing about a rate that can draw', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 20, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + }) + describe('traceSampleRate', () => { it('defaults to 100 if the option is not provided', () => { expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBe(100) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 33743b5d9e..657adabfd2 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -101,9 +101,10 @@ export interface RumInitConfiguration extends InitConfiguration { */ sessionReplaySampleRate?: number | undefined /** - * The percentage of tracked sessions that record a replay but only upload it if the session - * reports an error: 100 for all, 0 for none. Drawn only for sessions that the plain - * `sessionReplaySampleRate` draw missed, so a session is never counted by both rates. + * Of the tracked sessions that `sessionReplaySampleRate` did not draw, the percentage that record + * a replay but only upload it if the session reports an error: 100 for all of them, 0 for none. + * The base is what the plain rate missed, so a session is never counted by both, and the share of + * all tracked sessions this covers is `(100 - sessionReplaySampleRate) * this / 100`. * * Such a session records from the start and keeps at most the last minute of it in memory. If it * never reports an error, nothing is uploaded and the session is not billed. On the first error, @@ -244,6 +245,25 @@ export function validateAndBuildRumConfiguration( const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 + // Each of these is a rate the customer set that cannot draw a single session. They are valid + // numbers, so validation lets them through - but silence would leave them waiting for data that + // is never coming. + if (sessionReplayOnErrorSampleRate > 0) { + if (sessionReplaySampleRate === 100) { + display.warn( + 'sessionReplayOnErrorSampleRate is drawn only for sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' + ) + } + if ((initConfiguration.sessionSampleRate ?? 100) === 0) { + display.warn('sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0: no session is tracked.') + } + if (initConfiguration.startSessionReplayRecordingManually) { + display.warn( + 'sessionReplayOnErrorSampleRate needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' + ) + } + } + return { applicationId: initConfiguration.applicationId, version: initConfiguration.version || undefined, From 725cb021185b3f7e80cda524c27534d40f4a18d2 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:23:42 -0700 Subject: [PATCH 17/27] test(rum): hold the error-replay sampling to the promises it makes Every one of these covers a one-line change that would otherwise ship a feature that silently does nothing: the rate never reaching the built configuration, the recording not starting on its own, the release predicate inverted so no withheld replay is ever sent, the error mark no longer naming its session so the store refuses it, the internal buffer checkout reason leaking into the segment schema, the rate no longer range-checked, and the error-replay marker no longer emitted. Two fixtures were lying as well: non-error events carried an `error` object no real event has, which hid the order the guards have to read them in. --- .../configuration/configuration.spec.ts | 34 +++++++++++++++++++ .../domain/contexts/sessionContext.spec.ts | 18 ++++++++++ .../src/domain/trackSessionError.spec.ts | 9 +++-- packages/rum/src/boot/startRecording.spec.ts | 20 +++++++++++ .../segmentCollection.spec.ts | 5 ++- 5 files changed, 82 insertions(+), 4 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 6c39bb7cfa..bb01bf41c5 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -66,6 +66,37 @@ describe('validateAndBuildRumConfiguration', () => { }) describe('sessionReplayOnErrorSampleRate', () => { + it('is carried into the built configuration', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplayOnErrorSampleRate: 50 })! + .sessionReplayOnErrorSampleRate + ).toBe(50) + }) + + it('defaults to collecting no error replay at all', () => { + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.sessionReplayOnErrorSampleRate).toBe(0) + }) + + it('is rejected when it is not a sample rate', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplayOnErrorSampleRate: 'foo' as unknown as number, + }) + ).toBeUndefined() + expect(displayErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('starts the recording on its own, since there is nothing to withhold otherwise', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 0, + sessionReplayOnErrorSampleRate: 30, + })!.startSessionReplayRecordingManually + ).toBeFalse() + }) + it('warns when the plain replay rate leaves it nothing to draw from', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, @@ -74,6 +105,7 @@ describe('validateAndBuildRumConfiguration', () => { }) expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('sessionReplaySampleRate did not draw') }) it('warns when no session is tracked at all', () => { @@ -84,6 +116,7 @@ describe('validateAndBuildRumConfiguration', () => { }) expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('no session is tracked') }) it('warns when the recording is left for the customer to start, since nothing would be held', () => { @@ -94,6 +127,7 @@ describe('validateAndBuildRumConfiguration', () => { }) expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('startSessionReplayRecordingManually') }) it('says nothing about a rate that can draw', () => { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 3d72ad5199..d841fc2747 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -94,6 +94,24 @@ describe('session context', () => { expect(eventWithoutHasReplay.session!.has_replay).toBeUndefined() }) + it('should tell a replay kept only because the session errored apart from an unconditional one', () => { + sessionManager.setTrackedWithErrorSessionReplay() + const errorReplayEvent = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + sessionManager.setTrackedWithSessionReplay() + const plainEvent = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(errorReplayEvent.session!.sampled_for_error_replay).toBeTrue() + // absent rather than false, so it costs nothing on every ordinary session + expect(plainEvent.session!.sampled_for_error_replay).toBeUndefined() + }) + it('should not set hasReplay when a dropped buffer left the view with no segment', () => { // a withheld buffer that was dropped rolls its segments back, and a view left with zero of them // has no replay to offer however many records were once counted diff --git a/packages/rum-core/src/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts index 05b254496c..5cfe610038 100644 --- a/packages/rum-core/src/domain/trackSessionError.spec.ts +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -11,8 +11,10 @@ describe('startSessionErrorTracking', () => { let setSessionHasErrorSpy: jasmine.Spy function collect(type: string, source = 'source') { - lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type, error: { source } } as unknown as RumEvent & - Context) + // only error events carry an `error` object; anything else that did would hide a guard that + // reads it before checking the type + const event = type === 'error' ? { type, error: { source } } : { type } + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, event as unknown as RumEvent & Context) } beforeEach(() => { @@ -26,7 +28,8 @@ describe('startSessionErrorTracking', () => { it('marks the session on the first collected error', () => { collect('error') - expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + // named, not just counted: the mark is refused if it does not name the session it belongs to + expect(setSessionHasErrorSpy).toHaveBeenCalledOnceWith('session-id') }) it('leaves a session that withholds nothing alone, so an ordinary session store is never written', () => { diff --git a/packages/rum/src/boot/startRecording.spec.ts b/packages/rum/src/boot/startRecording.spec.ts index e1527e0a9d..a7342f185c 100644 --- a/packages/rum/src/boot/startRecording.spec.ts +++ b/packages/rum/src/boot/startRecording.spec.ts @@ -116,6 +116,26 @@ describe('startRecording', () => { expect(requests[0].metadata.records_count).toBe(1 + recordsPerFullSnapshot()) }) + it('sends the withheld replay once its session reports an error', async () => { + sessionManager.setTrackedWithErrorSessionReplay() + setupStartRecording() + + document.body.dispatchEvent(createNewEvent('click', { clientX: 1, clientY: 2 })) + // a page exit while the session is still waiting for an error keeps the buffer rather than + // sending it, so what follows joins the same segment + flushSegment(lifeCycle) + document.body.dispatchEvent(createNewEvent('click', { clientX: 3, clientY: 4 })) + + sessionManager.setSessionHasError() + flushSegment(lifeCycle) + + const requests = await readSentRequests(1) + expect(requestSendSpy).toHaveBeenCalledTimes(1) + // one segment, held since the recording started, carrying everything from before the error + expect(requests[0].metadata.creation_reason).toBe('init') + expect(requests[0].metadata.records_count).toBe(2 + recordsPerFullSnapshot()) + }) + it('drops a withheld replay when the session stops withholding without having errored', async () => { sessionManager.setTrackedWithErrorSessionReplay() setupStartRecording() diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index f765d45fd0..bd19e5d171 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -475,7 +475,10 @@ describe('startSegmentCollection withholding (error session replay)', () => { worker.processAllMessages() // the dropped buffer never reached the intake, so the first segment that does is index 0 - expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).index_in_view).toBe(0) + const metadata = await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0]) + expect(metadata.index_in_view).toBe(0) + // and it carries a reason the segment schema knows, not the internal one that dropped the buffer + expect(metadata.creation_reason).toBe('segment_duration_limit') }) it('does not restart the buffer when collection was stopped while the flush was in flight', () => { From dec56f03ee279021139728def1e8aa7cd5355f8a Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:45:32 -0700 Subject: [PATCH 18/27] refactor(rum): drop a withholding default nothing withholds by The segment collection took a buffering argument that defaulted to one that never withholds. Its only production caller always passes a real one, so the default existed for a single test that omitted the argument. That test now says what it means. --- .../segmentCollection/segmentCollection.spec.ts | 5 +++-- .../src/domain/segmentCollection/segmentCollection.ts | 11 ++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index bd19e5d171..c312a2f2fb 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -1,5 +1,5 @@ import type { ClocksState, HttpRequest, TimeStamp } from '@flashcatcloud/browser-core' -import { DeflateEncoderStreamId, PageExitReason } from '@flashcatcloud/browser-core' +import { DeflateEncoderStreamId, noop, PageExitReason } from '@flashcatcloud/browser-core' import type { ViewHistory, ViewHistoryEntry, RumConfiguration } from '@flashcatcloud/browser-rum-core' import { LifeCycle, LifeCycleEventType } from '@flashcatcloud/browser-rum-core' import type { Clock } from '@flashcatcloud/browser-core/test' @@ -72,7 +72,8 @@ describe('startSegmentCollection', () => { lifeCycle, () => context, httpRequestSpy, - createDeflateEncoder(configuration, worker, DeflateEncoderStreamId.REPLAY) + createDeflateEncoder(configuration, worker, DeflateEncoderStreamId.REPLAY), + { getWithholdingSessionId: () => undefined, isReleased: () => false, restartFromFullSnapshot: noop } )) registerCleanupTask(() => { diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 7953a09ffa..3e7346d935 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -4,7 +4,6 @@ import { isPageExitReason, ONE_SECOND, clearTimeout, - noop, relativeNow, setTimeout, } from '@flashcatcloud/browser-core' @@ -77,12 +76,6 @@ export interface SegmentBuffering { restartFromFullSnapshot: () => void } -const NO_BUFFERING: SegmentBuffering = { - getWithholdingSessionId: () => undefined, - isReleased: () => false, - restartFromFullSnapshot: noop, -} - export function startSegmentCollection( lifeCycle: LifeCycle, configuration: RumConfiguration, @@ -90,7 +83,7 @@ export function startSegmentCollection( viewHistory: ViewHistory, httpRequest: HttpRequest, encoder: DeflateEncoder, - buffering: SegmentBuffering = NO_BUFFERING + buffering: SegmentBuffering ) { return doStartSegmentCollection( lifeCycle, @@ -138,7 +131,7 @@ export function doStartSegmentCollection( getSegmentContext: () => SegmentContext | undefined, httpRequest: HttpRequest, encoder: DeflateEncoder, - buffering: SegmentBuffering = NO_BUFFERING + buffering: SegmentBuffering ) { let state: SegmentCollectionState = { status: SegmentCollectionStatus.WaitingForInitialRecord, From 06a261759e8944985f44fb72bf30bcf1822f6102 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:46:12 -0700 Subject: [PATCH 19/27] test(rum): name the session when marking it in the last spec that did not --- packages/rum-core/src/domain/rumSessionManager.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 438f188a6c..94e76fb1e4 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -290,7 +290,7 @@ describe('rum session manager', () => { expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() // still true once released, so what was stored can be told apart afterwards - sessionManager.setSessionHasError() + sessionManager.setSessionHasError(sessionManager.findTrackedSession()!.id) expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() }) From ba45ec6dc12e140577e10e49778213fef4962cb0 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 11:47:25 -0700 Subject: [PATCH 20/27] fix(rum): count records, not segments, when deciding a view has a replay Excluding a view whose withheld buffer was dropped was right, but counting segments to do it was not: when a host bridge takes the records there is never a segment to count, so a webview session - which cannot enable any of this - stopped reporting `has_replay` at all. Records are rolled back with the buffer they belonged to and are counted in every mode, so they answer both cases. --- .../domain/contexts/sessionContext.spec.ts | 20 +++++++++++++++---- .../src/domain/contexts/sessionContext.ts | 9 +++++---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index d841fc2747..5868240c9b 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -112,10 +112,10 @@ describe('session context', () => { expect(plainEvent.session!.sampled_for_error_replay).toBeUndefined() }) - it('should not set hasReplay when a dropped buffer left the view with no segment', () => { - // a withheld buffer that was dropped rolls its segments back, and a view left with zero of them - // has no replay to offer however many records were once counted - getReplayStatsSpy.and.returnValue({ ...fakeStats, segments_count: 0 }) + it('should not set hasReplay when a dropped buffer left the view with nothing', () => { + // a withheld buffer that was dropped rolls back what it held, and a view left with an empty + // stats entry has no replay to offer + getReplayStatsSpy.and.returnValue({ segments_count: 0, records_count: 0, segments_total_raw_size: 0 }) const event = hooks.triggerHook(HookNames.Assemble, { eventType: 'view', @@ -125,6 +125,18 @@ describe('session context', () => { expect(event.session!.has_replay).toBeUndefined() }) + it('should set hasReplay when a host bridge took the records and no segment was built', () => { + // records go straight to the bridge, so nothing ever counts a segment for them + getReplayStatsSpy.and.returnValue({ ...fakeStats, segments_count: 0, records_count: 10 }) + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.has_replay).toBe(true) + }) + it('should set session.is_active when the session is active', () => { findViewSpy.and.returnValue({ ...fakeView, sessionIsActive: true }) const eventWithActiveSession = hooks.triggerHook(HookNames.Assemble, { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index 289c33fa8e..aa0f84b4bf 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -29,11 +29,12 @@ export function startSessionContext( let sampledForErrorReplay let isActive if (eventType === RumEventType.VIEW) { - // Counted rather than merely present: a withheld buffer that was dropped rolls its segments - // back, which leaves a view with replay stats and no replay at all - and offering a replay - // that was never uploaded is worse than not offering one. + // Records rather than merely a stats entry: a withheld buffer that was dropped rolls back what + // it held, which leaves a view with an empty stats entry and no replay at all - and offering a + // replay that was never uploaded is worse than not offering one. Records, not segments, + // because a host bridge takes the records itself and no segment is ever built for them. const replayStats = recorderApi.getReplayStats(view.id) - hasReplay = !isReplayWithheld && replayStats && replayStats.segments_count > 0 ? true : undefined + hasReplay = !isReplayWithheld && replayStats && replayStats.records_count > 0 ? true : undefined sampledForReplay = session.sessionReplay === SessionReplayState.SAMPLED // Tells a replay collected only because the session errored apart from one collected // unconditionally - the two cost differently and are answered by different questions. From 89af49c05e548bbf3be86f2cdd39bb45baf61fda Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 5 Sep 2026 08:09:03 -0700 Subject: [PATCH 21/27] feat(rum): make sessionReplayOnError a switch, not a sample rate A replay kept only because the session errored answers "do I want to see this error's session" - and that is a yes or a no, not a share. Keeping a random half of the error replays would just leave half the reports uninvestigable, and the cost this could guard against is already bounded by sessionReplaySampleRate and by turning the option off. The rate also hid an arithmetic trap: it applied to whatever the plain rate missed, so the real share was (100 - sessionReplaySampleRate) * rate / 100, and a rate set next to a plain rate of 100 silently did nothing. A switch has nothing to multiply. `sessionReplayOnErrorSampleRate: number` becomes `sessionReplayOnError: boolean`, default false. The tracking types and the session cookie are unchanged: what was drawn is now simply applied. --- .../configuration/configuration.spec.ts | 37 +++++++++---------- .../src/domain/configuration/configuration.ts | 33 ++++++++--------- .../src/domain/rumSessionManager.spec.ts | 20 +++++----- .../rum-core/src/domain/rumSessionManager.ts | 6 +-- .../rum-core/src/domain/trackSessionError.ts | 2 +- .../segmentCollection/segmentCollection.ts | 2 +- 6 files changed, 48 insertions(+), 52 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index bb01bf41c5..685c4fd2a4 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -65,26 +65,25 @@ describe('validateAndBuildRumConfiguration', () => { }) }) - describe('sessionReplayOnErrorSampleRate', () => { + describe('sessionReplayOnError', () => { it('is carried into the built configuration', () => { expect( - validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplayOnErrorSampleRate: 50 })! - .sessionReplayOnErrorSampleRate - ).toBe(50) + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplayOnError: true })! + .sessionReplayOnError + ).toBeTrue() }) it('defaults to collecting no error replay at all', () => { - expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.sessionReplayOnErrorSampleRate).toBe(0) + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.sessionReplayOnError).toBeFalse() }) - it('is rejected when it is not a sample rate', () => { + it('is read as a switch, whatever it was given', () => { expect( validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, - sessionReplayOnErrorSampleRate: 'foo' as unknown as number, - }) - ).toBeUndefined() - expect(displayErrorSpy).toHaveBeenCalledTimes(1) + sessionReplayOnError: 1 as unknown as boolean, + })!.sessionReplayOnError + ).toBeTrue() }) it('starts the recording on its own, since there is nothing to withhold otherwise', () => { @@ -92,16 +91,16 @@ describe('validateAndBuildRumConfiguration', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplaySampleRate: 0, - sessionReplayOnErrorSampleRate: 30, + sessionReplayOnError: true, })!.startSessionReplayRecordingManually ).toBeFalse() }) - it('warns when the plain replay rate leaves it nothing to draw from', () => { + it('warns when the plain replay rate leaves it nothing to apply to', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplaySampleRate: 100, - sessionReplayOnErrorSampleRate: 50, + sessionReplayOnError: true, }) expect(displayWarnSpy).toHaveBeenCalledTimes(1) @@ -112,7 +111,7 @@ describe('validateAndBuildRumConfiguration', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionSampleRate: 0, - sessionReplayOnErrorSampleRate: 50, + sessionReplayOnError: true, }) expect(displayWarnSpy).toHaveBeenCalledTimes(1) @@ -122,7 +121,7 @@ describe('validateAndBuildRumConfiguration', () => { it('warns when the recording is left for the customer to start, since nothing would be held', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, - sessionReplayOnErrorSampleRate: 50, + sessionReplayOnError: true, startSessionReplayRecordingManually: true, }) @@ -130,11 +129,11 @@ describe('validateAndBuildRumConfiguration', () => { expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('startSessionReplayRecordingManually') }) - it('says nothing about a rate that can draw', () => { + it('says nothing about a switch that can apply', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplaySampleRate: 20, - sessionReplayOnErrorSampleRate: 50, + sessionReplayOnError: true, }) expect(displayWarnSpy).not.toHaveBeenCalled() @@ -608,7 +607,7 @@ describe('serializeRumConfiguration', () => { enablePrivacyForActionName: false, subdomain: 'foo', sessionReplaySampleRate: 60, - sessionReplayOnErrorSampleRate: 40, + sessionReplayOnError: true, startSessionReplayRecordingManually: true, trackUserInteractions: true, actionNameAttribute: 'test-id', @@ -635,7 +634,7 @@ describe('serializeRumConfiguration', () => { | 'profilingSampleRate' | 'propagateTraceBaggage' // not reported yet: needs a rum-events-format schema change first - | 'sessionReplayOnErrorSampleRate' + | 'sessionReplayOnError' ? never : CamelToSnakeCase // By specifying the type here, we can ensure that serializeConfiguration is returning an diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 657adabfd2..5085a039b7 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -101,16 +101,14 @@ export interface RumInitConfiguration extends InitConfiguration { */ sessionReplaySampleRate?: number | undefined /** - * Of the tracked sessions that `sessionReplaySampleRate` did not draw, the percentage that record - * a replay but only upload it if the session reports an error: 100 for all of them, 0 for none. - * The base is what the plain rate missed, so a session is never counted by both, and the share of - * all tracked sessions this covers is `(100 - sessionReplaySampleRate) * this / 100`. + * Whether the tracked sessions that `sessionReplaySampleRate` did not draw still record a replay, + * uploaded only if the session reports an error. Default: false. * * Such a session records from the start and keeps at most the last minute of it in memory. If it * never reports an error, nothing is uploaded and the session is not billed. On the first error, * the withheld minute is uploaded and recording continues normally for the rest of the session. */ - sessionReplayOnErrorSampleRate?: number | undefined + sessionReplayOnError?: boolean | undefined /** * If the session is sampled for Session Replay, only start the recording when `startSessionReplayRecording()` is called, instead of at the beginning of the session. Default: if startSessionReplayRecording is 0, true; otherwise, false. * See [Session Replay Usage](https://docs.datadoghq.com/real_user_monitoring/session_replay/browser/#usage) for further information. @@ -186,7 +184,7 @@ export interface RumConfiguration extends Configuration { defaultPrivacyLevel: DefaultPrivacyLevel enablePrivacyForActionName: boolean sessionReplaySampleRate: number - sessionReplayOnErrorSampleRate: number + sessionReplayOnError: boolean startSessionReplayRecordingManually: boolean trackUserInteractions: boolean trackViewsManually: boolean @@ -219,7 +217,6 @@ export function validateAndBuildRumConfiguration( if ( !isSampleRate(initConfiguration.sessionReplaySampleRate, 'Session Replay') || - !isSampleRate(initConfiguration.sessionReplayOnErrorSampleRate, 'Session Replay on Error') || !isSampleRate(initConfiguration.traceSampleRate, 'Trace') ) { return @@ -243,23 +240,23 @@ export function validateAndBuildRumConfiguration( const profilingEnabled = isExperimentalFeatureEnabled(ExperimentalFeature.PROFILING) const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 - const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 + const sessionReplayOnError = !!initConfiguration.sessionReplayOnError - // Each of these is a rate the customer set that cannot draw a single session. They are valid - // numbers, so validation lets them through - but silence would leave them waiting for data that - // is never coming. - if (sessionReplayOnErrorSampleRate > 0) { + // Each of these is a combination the customer can set that cannot apply to a single session. It + // is valid, so validation lets it through - but silence would leave them waiting for data that is + // never coming. + if (sessionReplayOnError) { if (sessionReplaySampleRate === 100) { display.warn( - 'sessionReplayOnErrorSampleRate is drawn only for sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' + 'sessionReplayOnError only applies to sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' ) } if ((initConfiguration.sessionSampleRate ?? 100) === 0) { - display.warn('sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0: no session is tracked.') + display.warn('sessionReplayOnError has no effect while sessionSampleRate is 0: no session is tracked.') } if (initConfiguration.startSessionReplayRecordingManually) { display.warn( - 'sessionReplayOnErrorSampleRate needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' + 'sessionReplayOnError needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' ) } } @@ -269,13 +266,13 @@ export function validateAndBuildRumConfiguration( version: initConfiguration.version || undefined, actionNameAttribute: initConfiguration.actionNameAttribute, sessionReplaySampleRate, - sessionReplayOnErrorSampleRate, + sessionReplayOnError, startSessionReplayRecordingManually: initConfiguration.startSessionReplayRecordingManually !== undefined ? !!initConfiguration.startSessionReplayRecordingManually : // An error-sampled session has to be recording before the error happens, otherwise there is // nothing to withhold and release. So it must auto-start just like a plain sampled one. - sessionReplaySampleRate === 0 && sessionReplayOnErrorSampleRate === 0, + sessionReplaySampleRate === 0 && !sessionReplayOnError, traceSampleRate: initConfiguration.traceSampleRate ?? 100, rulePsr: isNumber(initConfiguration.traceSampleRate) ? initConfiguration.traceSampleRate / 100 : undefined, allowedTracingUrls, @@ -361,7 +358,7 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) { return { session_replay_sample_rate: configuration.sessionReplaySampleRate, - // `session_replay_on_error_sample_rate` is deliberately not reported yet: the telemetry + // `session_replay_on_error` is deliberately not reported yet: the telemetry // configuration type is generated from the rum-events-format schema, so adding it needs a schema // change first, and that is a separate repository. start_session_replay_recording_manually: configuration.startSessionReplayRecordingManually, diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 94e76fb1e4..5c636d3fc9 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -213,15 +213,15 @@ describe('rum session manager', () => { describe('error session replay sampling', () => { it('draws the error-replay type only when the plain replay draw missed', () => { startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100, sessionReplayOnError: true }, }) expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) }) - it('stores the error-replay type when only that rate is hit', () => { + it('stores the error-replay type when only the switch applies', () => { startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, }) expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( @@ -231,7 +231,7 @@ describe('rum session manager', () => { it('withholds the replay until the session reports an error', () => { const sessionManager = startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, }) expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) @@ -243,7 +243,7 @@ describe('rum session manager', () => { it('does not mark a session that has since been replaced by another one', () => { const sessionManager = startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, }) // another tab renewed the session while the mark was on its way to the store @@ -259,7 +259,7 @@ describe('rum session manager', () => { pending('the store lock, and so a deferred write, only exists on Chromium') } const sessionManager = startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, }) const sessionId = sessionManager.findTrackedSession()!.id @@ -284,7 +284,7 @@ describe('rum session manager', () => { it('marks the session so a replay kept only because it errored can be told apart', () => { const sessionManager = startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, }) expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() @@ -305,7 +305,7 @@ describe('rum session manager', () => { it('releases the replay when it is forced, rather than waiting for an error that may never come', () => { const sessionManager = startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, }) expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) @@ -314,9 +314,9 @@ describe('rum session manager', () => { expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.FORCED) }) - it('tracks the session even when no replay rate is hit at all', () => { + it('tracks the session even when neither the replay rate nor the switch applies', () => { const sessionManager = startRumSessionManagerWithDefaults({ - configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 0 }, + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: false }, }) expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY) diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index ef6bcc4018..75877afd4d 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -26,7 +26,7 @@ export interface RumSessionManager { setForcedReplay: () => void /** * Marks the given session as having reported an error. For a session sampled by - * `sessionReplayOnErrorSampleRate`, this is what releases the withheld replay. The id is required + * `sessionReplayOnError`, this is what releases the withheld replay. The id is required * because the store write can be deferred by the lock, and it must not land on a later session. */ setSessionHasError: (sessionId: string) => void @@ -179,8 +179,8 @@ function computeSessionState(configuration: RumConfiguration, rawTrackingType?: trackingType = RumTrackingType.NOT_TRACKED } else if (performDraw(configuration.sessionReplaySampleRate)) { trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY - } else if (performDraw(configuration.sessionReplayOnErrorSampleRate)) { - // Drawn only when the plain replay draw missed, so a session is never counted by both rates. + } else if (configuration.sessionReplayOnError) { + // Only for sessions the plain replay draw missed, so a session is never counted by both. trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY } else { trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts index 531d7f6aa5..2fd0e2dcc6 100644 --- a/packages/rum-core/src/domain/trackSessionError.ts +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -6,7 +6,7 @@ import type { RumSessionManager } from './rumSessionManager' /** * Marks the session as having reported an error, which is what releases a replay withheld by - * `sessionReplayOnErrorSampleRate`. + * `sessionReplayOnError`. * * It listens after assembly rather than on the raw error, so an error discarded by `beforeSend` or * by a rate limiter does not release anything: a session billed for an error that cannot be found diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 3e7346d935..605cf68a31 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -56,7 +56,7 @@ export let SEGMENT_BYTES_LIMIT = 60_000 /** * Lets a session record without uploading anything until it reports an error. Sessions drawn by - * `sessionReplayOnErrorSampleRate` record from the start, but every segment is withheld: dropped on + * `sessionReplayOnError` record from the start, but every segment is withheld: dropped on * checkout while no error has happened, sent normally from the moment one has. */ export interface SegmentBuffering { From 7aef035aec8076e7ba31fbae6eb28e33b739fd87 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 5 Sep 2026 08:17:21 -0700 Subject: [PATCH 22/27] test(rum): name the session replay on error specs after the switch --- packages/rum-core/src/domain/rumSessionManager.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 5c636d3fc9..a160bead28 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -210,8 +210,8 @@ describe('rum session manager', () => { ) }) - describe('error session replay sampling', () => { - it('draws the error-replay type only when the plain replay draw missed', () => { + describe('session replay on error', () => { + it('applies the error-replay type only when the plain replay draw missed', () => { startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100, sessionReplayOnError: true }, }) From 378981eb141801cdffc48864e96ee92a30024fb2 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 5 Sep 2026 17:37:30 -0700 Subject: [PATCH 23/27] fix(rum): narrow the next creation reason where the compiler can see it --- .../segmentCollection/segmentCollection.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 33e357b96a..4629bbd332 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -238,7 +238,12 @@ export function doStartSegmentCollection( if (flushReason !== 'stop') { state = { status: SegmentCollectionStatus.WaitingForInitialRecord, - nextSegmentCreationReason: toCreationReason(flushReason), + nextSegmentCreationReason: + flushReason === 'buffer_checkout' + ? 'segment_duration_limit' + : flushReason === 'page_reactivated' + ? 'view_change' + : flushReason, } } else { state = { @@ -318,17 +323,6 @@ export function doStartSegmentCollection( } } -function toCreationReason(flushReason: InternalFlushReason): CreationReason { - switch (flushReason) { - case 'buffer_checkout': - return 'segment_duration_limit' - case 'page_reactivated': - return 'view_change' - default: - return flushReason - } -} - export function computeSegmentContext( applicationId: string, sessionManager: RumSessionManager, From a83b13d8caedc961c0c7134db4db48be3e3bbab3 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 5 Sep 2026 17:40:48 -0700 Subject: [PATCH 24/27] test(rum): store the released session with the expiry a session now has to carry --- packages/rum-core/src/domain/rumSessionManager.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index ecca623481..194f77f342 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -1261,7 +1261,11 @@ describe('rum session manager', () => { }) it('keeps the released state across a page load, since it is persisted in the session store', () => { - setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=3&hasError=1', DURATION) + setCookie( + SESSION_STORE_KEY, + `id=abcdef&rum=3&hasError=1&created=${Date.now()}&expire=${Date.now() + DURATION}`, + DURATION + ) const sessionManager = startRumSessionManagerWithDefaults() From 64132274326ca49d1bc0980581f3d7cc51369957 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 6 Sep 2026 19:18:54 -0700 Subject: [PATCH 25/27] feat(rum): read sessionReplayOnError from remote configuration The console can now deliver the switch beside the rates, so an operator turns error replays on or off without shipping a release. It is read at the draw like the rates and latched the same way: a session either withholds its replay from the start or never does. `beforeSampling` is not offered it - a switch is a yes or a no the console already answered. A delivered value that is not a boolean is dropped, so it reads as "not delivered" rather than as either position. --- .../configuration/remoteConfiguration.spec.ts | 24 +++++++++++++ .../configuration/remoteConfiguration.ts | 18 ++++++++++ .../src/domain/rumSessionManager.spec.ts | 35 +++++++++++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 19 +++++++--- 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index c3e409ab8c..1bc0b3fd24 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -121,6 +121,30 @@ describe('remoteConfiguration', () => { start(configurationWith()) }) + it('keeps the replay-on-error switch the server reports, either way it is set', (done) => { + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionReplaySampleRate: 10, sessionReplayOnError: false } })) + + expect(readRemoteConfig(setup)).toEqual({ + sessionReplaySampleRate: 10, + sessionReplayOnError: false, + version: 3, + }) + done() + }) + start(configurationWith()) + }) + + it('drops a switch that is not a boolean, so it reads as not delivered', (done) => { + interceptor.withMockXhr((xhr) => { + xhr.complete(200, body({ rum: { sessionSampleRate: 50, sessionReplayOnError: 'true' as unknown as boolean } })) + + expect(readRemoteConfig(setup)).toEqual({ sessionSampleRate: 50, version: 3 }) + done() + }) + start(configurationWith()) + }) + it('drops a privacy level it does not recognise rather than passing it on', (done) => { // A typo must not reach the recorders: an unknown value there falls through to recording // everything, which is the one outcome nobody asks for by accident. diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 2c0ee781f9..24be706f18 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -102,6 +102,12 @@ export interface RemoteConfigValues { * fact. */ defaultPrivacyLevel?: DefaultPrivacyLevel + /** + * Whether the sessions `sessionReplaySampleRate` did not draw still record a replay, uploaded only + * if the session errors. Read at the draw like the rates, and for the same reason: a session + * either withholds its replay from the start or never does. + */ + sessionReplayOnError?: boolean /** * Which version of the settings these rates came from. Reported back on the next request so the * console can say how far a change has actually reached — a question the events cannot answer, @@ -261,6 +267,9 @@ function readStoredValues(parsed: unknown): RemoteConfigValues { if (isPrivacyLevel(stored.defaultPrivacyLevel)) { values.defaultPrivacyLevel = stored.defaultPrivacyLevel } + if (isSwitch(stored.sessionReplayOnError)) { + values.sessionReplayOnError = stored.sessionReplayOnError + } if (isBag(stored.custom)) { values.custom = stored.custom } @@ -484,6 +493,11 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) if (isPrivacyLevel(response.rum.defaultPrivacyLevel)) { values.defaultPrivacyLevel = response.rum.defaultPrivacyLevel } + // A switch is a boolean or nothing. Anything else - a "true" string, a 1 - is dropped for the + // same reason a bad rate is: it must read as "not delivered", not as either position. + if (isSwitch(response.rum.sessionReplayOnError)) { + values.sessionReplayOnError = response.rum.sessionReplayOnError + } } // The custom bag rides along untouched — the platform's job is delivery, its meaning belongs to // the host application. Gone from the response (or the kill switch off) means gone from storage. @@ -732,6 +746,10 @@ export function isRate(value: unknown): value is number { return typeof value === 'number' && value >= 0 && value <= 100 } +export function isSwitch(value: unknown): value is boolean { + return typeof value === 'boolean' +} + /** * A version is a publish counter, so anything that is not a whole, non-negative number small enough * to survive a JSON round trip cannot be one. Checked on the way in and on the way out, because a diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 194f77f342..5543765f4d 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -229,6 +229,7 @@ describe('rum session manager', () => { sessionReplaySampleRate?: number traceSampleRate?: number defaultPrivacyLevel?: string + sessionReplayOnError?: boolean }) { localStorage.setItem(STORE_KEY, JSON.stringify(values)) registerCleanupTask(() => localStorage.removeItem(STORE_KEY)) @@ -256,6 +257,40 @@ describe('rum session manager', () => { expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) }) + it('keeps a replay on error when the console says so, over what init said', () => { + storeRemoteConfigValues({ sessionReplaySampleRate: 0, sessionReplayOnError: true }) + + startRumSessionManagerWithDefaults({ + configuration: { + sessionSampleRate: 100, + sessionReplaySampleRate: 100, + sessionReplayOnError: false, + remoteConfig: REMOTE_SAMPLING_SETUP, + }, + }) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + ) + }) + + it('turns the replay-on-error switch off when the console says so', () => { + storeRemoteConfigValues({ sessionReplayOnError: false }) + + startRumSessionManagerWithDefaults({ + configuration: { + sessionSampleRate: 100, + sessionReplaySampleRate: 0, + sessionReplayOnError: true, + remoteConfig: REMOTE_SAMPLING_SETUP, + }, + }) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY) + }) + it('falls back to the rate passed to init for a knob the console did not set', () => { storeRemoteConfigValues({ sessionReplaySampleRate: 100 }) diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 512e9d9825..f44b80fde6 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -541,7 +541,10 @@ function computeSessionState( // the decision it was created with: settings arriving mid-session never start or stop // collecting for a visitor already on the site. const remote = readRemoteConfig(configuration.remoteConfig) - const { sessionSampleRate, sessionReplaySampleRate } = resolveSampleRates(configuration, remote) + const { sessionSampleRate, sessionReplaySampleRate, sessionReplayOnError } = resolveSampleRates( + configuration, + remote + ) reportDraw(configuration, remote, sessionSampleRate, sessionReplaySampleRate, onDraw) @@ -549,7 +552,7 @@ function computeSessionState( trackingType = RumTrackingType.NOT_TRACKED } else if (performDraw(sessionReplaySampleRate)) { trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY - } else if (configuration.sessionReplayOnError) { + } else if (sessionReplayOnError) { // Only for sessions the plain replay draw missed, so a session is never counted by both. trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY } else { @@ -563,8 +566,10 @@ function computeSessionState( } /** - * FLASHCAT FORK - the rates a draw would use right now: what the console delivered, falling back to - * what the site passed to init, with the application's `beforeSampling` given the last word. This + * FLASHCAT FORK - the rates a draw would use right now, and the on-error switch beside them: what + * the console delivered, falling back to what the site passed to init, with the application's + * `beforeSampling` given the last word on the rates (the switch is not offered to it: it is a + * yes or a no the console already answered). This * is what turns the delivered custom values into sampling decisions without a wasted first draw or * a session restart: the console ships the data (an allow-list, a cohort rule), the application's * own code interprets it here. Its failure modes must never reach session creation, so a thrown @@ -598,7 +603,11 @@ function resolveSampleRates(configuration: RumConfiguration, remote: RemoteConfi } } - return { sessionSampleRate, sessionReplaySampleRate } + return { + sessionSampleRate, + sessionReplaySampleRate, + sessionReplayOnError: remote.sessionReplayOnError ?? configuration.sessionReplayOnError, + } } /** From 305413563ad1fbb5583bab9de79f939a655ee1dd Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Sep 2026 02:48:49 -0700 Subject: [PATCH 26/27] fix(rum): restore remotely enabled replay and oversized snapshot baselines --- .../configuration/configuration.spec.ts | 20 +++++ .../src/domain/configuration/configuration.ts | 5 +- packages/rum/src/boot/recorderApi.spec.ts | 35 ++++++++ .../segmentCollection.spec.ts | 86 +++++++++++++++++++ .../segmentCollection/segmentCollection.ts | 53 ++++++++++-- 5 files changed, 192 insertions(+), 7 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 5f82b9dfca..20f89eb359 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -358,6 +358,26 @@ describe('validateAndBuildRumConfiguration', () => { }) describe('startSessionReplayRecordingManually', () => { + it('keeps automatic recording available for remotely enabled replay', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 0, + remoteConfigurationEnabled: true, + })!.startSessionReplayRecordingManually + ).toBeFalse() + }) + + it('respects explicit manual recording when remote configuration is enabled', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + remoteConfigurationEnabled: true, + startSessionReplayRecordingManually: true, + })!.startSessionReplayRecordingManually + ).toBeTrue() + }) + it('defaults to true if sessionReplaySampleRate is 0', () => { expect( validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplaySampleRate: 0 })! diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 73290133b1..0467f75a5f 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -426,8 +426,9 @@ export function validateAndBuildRumConfiguration( initConfiguration.startSessionReplayRecordingManually !== undefined ? !!initConfiguration.startSessionReplayRecordingManually : // An error-sampled session has to be recording before the error happens, otherwise there is - // nothing to withhold and release. So it must auto-start just like a plain sampled one. - sessionReplaySampleRate === 0 && !sessionReplayOnError, + // nothing to withhold and release. Remote configuration may enable replay on a later + // session, so keep the automatic start intent even when init disables replay. + sessionReplaySampleRate === 0 && !sessionReplayOnError && !initConfiguration.remoteConfigurationEnabled, sessionReplayDirectUpload: !!initConfiguration.sessionReplayDirectUpload, traceSampleRate: initConfiguration.traceSampleRate ?? 100, rulePsr: isNumber(initConfiguration.traceSampleRate) ? initConfiguration.traceSampleRate / 100 : undefined, diff --git a/packages/rum/src/boot/recorderApi.spec.ts b/packages/rum/src/boot/recorderApi.spec.ts index f5206f2766..f0db0dda63 100644 --- a/packages/rum/src/boot/recorderApi.spec.ts +++ b/packages/rum/src/boot/recorderApi.spec.ts @@ -10,6 +10,7 @@ import { mockRumConfiguration, mockViewHistory, } from '../../../rum-core/test' +import { validateAndBuildRumConfiguration } from '../../../rum-core/src/domain/configuration' import type { CreateDeflateWorker } from '../domain/deflate' import { MockWorker } from '../../test' import { resetDeflateWorkerState } from '../domain/deflate' @@ -73,6 +74,40 @@ describe('makeRecorderApi', () => { } describe('recorder boot', () => { + it('starts a remotely selected buffered replay with the built recording default', async () => { + const configuration = validateAndBuildRumConfiguration({ + applicationId: 'app', + clientToken: 'token', + remoteConfigurationEnabled: true, + })! + setupRecorderApi({ + sessionManager: createRumSessionManagerMock().setTrackedWithErrorSessionReplay(), + startSessionReplayRecordingManually: configuration.startSessionReplayRecordingManually, + }) + rumInit() + expect(loadRecorderSpy).toHaveBeenCalledTimes(1) + await collectAsyncCalls(startRecordingSpy, 1) + }) + + it('keeps automatic start intent until a later session enables buffered replay', async () => { + const configuration = validateAndBuildRumConfiguration({ + applicationId: 'app', + clientToken: 'token', + remoteConfigurationEnabled: true, + })! + const sessionManager = createRumSessionManagerMock().setNotTracked() + setupRecorderApi({ + sessionManager, + startSessionReplayRecordingManually: configuration.startSessionReplayRecordingManually, + }) + rumInit() + expect(loadRecorderSpy).not.toHaveBeenCalled() + sessionManager.setTrackedWithErrorSessionReplay() + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + expect(loadRecorderSpy).toHaveBeenCalledTimes(1) + await collectAsyncCalls(startRecordingSpy, 1) + }) + describe('with automatic start', () => { it('starts recording when init() is called', async () => { setupRecorderApi() diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index 87e15d445b..af41d49d3f 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -454,6 +454,92 @@ describe('startSegmentCollection withholding (error session replay)', () => { expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(2) }) + it('restores a full snapshot after consecutive oversized snapshots and an error', async () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(VERY_BIG_RECORD)) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + + reportError() + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalled() + expect( + (await readMetadataFromReplayPayload(httpRequestSpy.send.calls.first().args[0])).has_full_snapshot + ).toBeTrue() + }) + + it('restores a missing snapshot before an errored page exits during the restart delay', async () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(VERY_BIG_RECORD)) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + reportError() + addRecord(RECORD) + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + worker.processAllMessages() + + expect(httpRequestSpy.sendOnExit).toHaveBeenCalled() + expect( + (await readMetadataFromReplayPayload(httpRequestSpy.sendOnExit.calls.first().args[0])).has_full_snapshot + ).toBeTrue() + }) + + it('restores the missing snapshot as soon as an error releases the session', () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(VERY_BIG_RECORD)) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + reportError() + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: 'error' } as any) + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(2) + worker.processAllMessages() + expect(httpRequestSpy.send).toHaveBeenCalled() + }) + + it('cancels the delayed replacement when a new view supplies a snapshot', () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(VERY_BIG_RECORD)) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {} as any) + addRecord({ ...VERY_BIG_RECORD, data: {} } as BrowserRecord) + worker.processAllMessages() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + expect(httpRequestSpy.send).not.toHaveBeenCalled() + }) + + it('does not repeatedly serialize an oversized document while waiting for an error', () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(VERY_BIG_RECORD)) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + for (let i = 0; i < 4; i++) { + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + } + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + expect(httpRequestSpy.send).not.toHaveBeenCalled() + }) + + it('cancels a delayed snapshot when recording stops', () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(VERY_BIG_RECORD)) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + stopCollection() + clock.tick(SEGMENT_DURATION_LIMIT * 2) + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + expect(httpRequestSpy.send).not.toHaveBeenCalled() + }) + it('keeps the buffer when the page is only hidden, so the replay can still start from its snapshot', () => { // switching tabs is ordinary; dropping here would take the only full snapshot with it addRecord(RECORD) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 4629bbd332..1319d1102c 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -10,6 +10,7 @@ import { import type { LifeCycle, ViewHistory, RumSessionManager, RumConfiguration } from '@flashcatcloud/browser-rum-core' import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' +import { RecordType } from '../../types' import { discardSegmentData, removeSegment } from '../replayStats' import { buildReplayPayload } from './buildReplayPayload' import type { FlushReason, Segment } from './segment' @@ -143,6 +144,7 @@ export function doStartSegmentCollection( // back up to a minute" is a promise nobody can check. let droppedBufferCount = 0 let lastBufferRestartAt: RelativeTime | undefined + let bufferRestartTimeoutId: TimeoutId | undefined const { unsubscribe: unsubscribeViewCreated } = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, () => { flushSegment('view_change') @@ -162,7 +164,36 @@ export function doStartSegmentCollection( flushSegment('page_reactivated') }) + const { unsubscribe: unsubscribeRumEvent } = lifeCycle.subscribe( + LifeCycleEventType.RUM_EVENT_COLLECTED, + restoreReleasedSnapshot + ) + + function restoreReleasedSnapshot() { + if (bufferRestartTimeoutId === undefined) { + return + } + const context = getSegmentContext() + if (context && buffering.isReleased(context.session.id)) { + // The error tracker marks the session before this listener runs. Restore the missing + // baseline now, before a view change or page exit can flush an unplayable segment. + clearTimeout(bufferRestartTimeoutId) + bufferRestartTimeoutId = undefined + lastBufferRestartAt = relativeNow() + buffering.restartFromFullSnapshot() + } else { + // The same oversized snapshot would be discarded again. Poll only for a release, without + // repeatedly serializing the document when neither an error nor new activity has arrived. + clearTimeout(bufferRestartTimeoutId) + bufferRestartTimeoutId = setTimeout(restoreReleasedSnapshot, SEGMENT_DURATION_LIMIT) + } + } + function flushSegment(flushReason: InternalFlushReason) { + if (flushReason !== 'view_change' && flushReason !== 'page_reactivated') { + // A release can also arrive through the shared session store without a local error event. + restoreReleasedSnapshot() + } // Decided once, and against the session that produced the records rather than whatever session // is current now: a segment must be either dropped or sent as a whole. const withheldForSessionId = @@ -266,12 +297,15 @@ export function doStartSegmentCollection( // to stop, and count records into the replay stats that no segment will ever hold. return } - // On a document whose full snapshot alone exceeds the segment limit, every restart would blow - // the limit again straight away and restart once more. Spacing restarts out avoids that hot - // loop, at the cost of a buffer that carries no full snapshot until the next restart is allowed - // - if the error lands in that window, what is released cannot be played from its start. + // A snapshot can itself exceed the budget. After a rapid second discard, wait for release + // before replacing it: ordinary flushes no longer restart buffers once the session errors. + clearTimeout(bufferRestartTimeoutId) + bufferRestartTimeoutId = undefined const now = relativeNow() - if (lastBufferRestartAt === undefined || now - lastBufferRestartAt >= SEGMENT_DURATION_LIMIT) { + const delay = lastBufferRestartAt === undefined ? 0 : SEGMENT_DURATION_LIMIT - (now - lastBufferRestartAt) + if (delay > 0) { + bufferRestartTimeoutId = setTimeout(restoreReleasedSnapshot, delay) + } else { lastBufferRestartAt = now buffering.restartFromFullSnapshot() } @@ -283,6 +317,12 @@ export function doStartSegmentCollection( return } + if (record.type === RecordType.FullSnapshot) { + // A view change or page reactivation can supply the replacement before the timer does. + clearTimeout(bufferRestartTimeoutId) + bufferRestartTimeoutId = undefined + } + if (state.status === SegmentCollectionStatus.WaitingForInitialRecord) { const context = getSegmentContext() if (!context) { @@ -316,9 +356,12 @@ export function doStartSegmentCollection( stop: () => { flushSegment('stop') + clearTimeout(bufferRestartTimeoutId) + bufferRestartTimeoutId = undefined unsubscribeViewCreated() unsubscribePageMayExit() unsubscribeReactivated() + unsubscribeRumEvent() }, } } From 2dceabffed5a58796aa7caac649bfe96797ec96a Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Sep 2026 05:02:39 -0700 Subject: [PATCH 27/27] fix(rum): preserve conditional replay across session and worker races --- .../session/sessionStoreOperations.spec.ts | 3 + .../domain/session/sessionStoreOperations.ts | 2 + .../core/src/domain/telemetry/telemetry.ts | 4 +- packages/rum-core/src/domain/lifeCycle.ts | 4 + .../src/domain/rumSessionManager.spec.ts | 70 +++++++ .../rum-core/src/domain/rumSessionManager.ts | 58 ++++-- .../src/domain/trackSessionError.spec.ts | 21 ++- .../rum-core/src/domain/trackSessionError.ts | 2 +- .../src/domain/sessionStore.spec.ts | 25 +++ .../rum-legacy/src/domain/sessionStore.ts | 15 +- packages/rum/README.md | 32 ++++ packages/rum/src/domain/replayStats.ts | 4 +- .../segmentCollection.spec.ts | 104 ++++++++++ .../segmentCollection/segmentCollection.ts | 177 ++++++++++++------ 14 files changed, 441 insertions(+), 80 deletions(-) diff --git a/packages/core/src/domain/session/sessionStoreOperations.spec.ts b/packages/core/src/domain/session/sessionStoreOperations.spec.ts index 78d76b6efe..37a8ab374f 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.spec.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.spec.ts @@ -1,3 +1,4 @@ +import { startFakeTelemetry } from '../telemetry' import type { MockStorage } from '../../../test' import { mockClock, mockCookie, mockLocalStorage } from '../../../test' import type { CookieOptions } from '../../browser/cookie' @@ -232,6 +233,7 @@ const DEFAULT_INIT_CONFIGURATION = { trackAnonymousUser: true } as Configuration it('should abort after a max number of retry', () => { const clock = mockClock() + const telemetry = startFakeTelemetry() sessionStoreStrategy.persistSession(initialSession) storage.setSpy.calls.reset() @@ -246,6 +248,7 @@ const DEFAULT_INIT_CONFIGURATION = { trackAnonymousUser: true } as Configuration expect(processSpy).not.toHaveBeenCalled() expect(afterSpy).not.toHaveBeenCalled() expect(storage.setSpy).not.toHaveBeenCalled() + expect(telemetry).toContain(jasmine.objectContaining({ message: 'Session store lock retries exhausted' })) clock.cleanup() }) diff --git a/packages/core/src/domain/session/sessionStoreOperations.ts b/packages/core/src/domain/session/sessionStoreOperations.ts index 869347d0cc..5be4d4cc5f 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.ts @@ -1,3 +1,4 @@ +import { addTelemetryDebug } from '../telemetry' import { setTimeout } from '../../tools/timer' import { generateUUID } from '../../tools/utils/stringUtils' import type { SessionStoreStrategy } from './storeStrategies/sessionStoreStrategy' @@ -43,6 +44,7 @@ export function processSessionStoreOperations( return } if (isLockEnabled && numberOfRetries >= LOCK_MAX_TRIES) { + addTelemetryDebug('Session store lock retries exhausted', { retries: numberOfRetries }) next(sessionStoreStrategy) return } diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index 4b0a12c617..4651ebb253 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -4,7 +4,9 @@ import { NO_ERROR_STACK_PRESENT_MESSAGE, isError } from '../error/error' import { toStackTraceString } from '../../tools/stackTrace/handlingStack' import { getExperimentalFeatures } from '../../tools/experimentalFeatures' import type { Configuration } from '../configuration' -import { INTAKE_SITE_STAGING } from '../configuration' +// Import the constant without loading configuration construction, which uses session storage. +// eslint-disable-next-line local-rules/disallow-protected-directory-import +import { INTAKE_SITE_STAGING } from '../configuration/intakeSites' import { Observable } from '../../tools/observable' import { timeStampNow } from '../../tools/utils/timeUtils' import { displayIfDebugEnabled, startMonitorErrorCollection } from '../../tools/monitor' diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index 78b6d9fccb..9f65158973 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -49,6 +49,8 @@ export const enum LifeCycleEventType { // at the end leaves upstream's numbering alone and keeps this file out of the way of the next // upstream merge. REMOTE_CONFIGURATION_STORED, + /** A local or shared session mark has released conditional collection. */ + SESSION_RELEASED, } // This is a workaround for an issue occurring when the Browser SDK is included in a TypeScript @@ -81,6 +83,7 @@ declare const LifeCycleEventTypeAsConst: { RAW_RUM_EVENT_COLLECTED: LifeCycleEventType.RAW_RUM_EVENT_COLLECTED RUM_EVENT_COLLECTED: LifeCycleEventType.RUM_EVENT_COLLECTED RAW_ERROR_COLLECTED: LifeCycleEventType.RAW_ERROR_COLLECTED + SESSION_RELEASED: LifeCycleEventType.SESSION_RELEASED REMOTE_CONFIGURATION_STORED: LifeCycleEventType.REMOTE_CONFIGURATION_STORED } @@ -106,6 +109,7 @@ export interface LifeCycleEventMap { error: RawError customerContext?: Context } + [LifeCycleEventTypeAsConst.SESSION_RELEASED]: { sessionId: string; reason: 'error' | 'force' } [LifeCycleEventTypeAsConst.REMOTE_CONFIGURATION_STORED]: void } diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 5543765f4d..0fd2bf83f0 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -1232,6 +1232,76 @@ describe('rum session manager', () => { }) describe('session replay on error', () => { + for (const mark of ['error', 'force'] as const) { + for (const replacement of [false, true]) { + it(`reconciles ${mark} after lock exhaustion only for its original session (replacement=${replacement})`, () => { + if (!isChromium()) { + pending('requires a cookie store lock') + } + const manager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, + }) + const id = manager.findTrackedSession()!.id + const state = `id=${id}&rum=3&created=${Date.now()}&expire=${Date.now() + DURATION}` + setCookie(SESSION_STORE_KEY, `lock=other-tab&${state}`, DURATION) + if (mark === 'error') { + manager.setSessionHasError(id) + } else { + manager.setForcedReplay() + } + clock.tick(1500) + setCookie(SESSION_STORE_KEY, replacement ? state.replace(id, 'replacement') : state, DURATION) + clock.tick(3000) + expect(getSessionState(SESSION_STORE_KEY)[mark === 'error' ? 'hasError' : 'forcedReplay']).toBe( + replacement ? undefined : '1' + ) + }) + } + } + + for (const force of ['setForcedReplay', 'setForcedSession'] as const) { + it(`${force} releases in memory before a locked store can persist it`, () => { + if (!isChromium()) { + pending('requires a cookie store lock') + } + const manager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, + }) + const id = manager.findTrackedSession()!.id + setCookie( + SESSION_STORE_KEY, + `lock=other-tab&id=${id}&rum=3&created=${Date.now()}&expire=${Date.now() + DURATION}`, + DURATION + ) + manager[force]() + expect(manager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.FORCED) + expect(getSessionState(SESSION_STORE_KEY).forcedReplay).toBeUndefined() + }) + + it(`${force} never writes its deferred mark into a replacement session`, () => { + if (!isChromium()) { + pending('requires a cookie store lock') + } + const manager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnError: true }, + }) + const id = manager.findTrackedSession()!.id + setCookie( + SESSION_STORE_KEY, + `lock=other-tab&id=${id}&rum=3&created=${Date.now()}&expire=${Date.now() + DURATION}`, + DURATION + ) + manager[force]() + setCookie( + SESSION_STORE_KEY, + `id=replacement&rum=3&created=${Date.now()}&expire=${Date.now() + DURATION}`, + DURATION + ) + clock.tick(20) + expect(getSessionState(SESSION_STORE_KEY).forcedReplay).toBeUndefined() + }) + } + it('applies the error-replay type only when the plain replay draw missed', () => { startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100, sessionReplayOnError: true }, diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index f44b80fde6..898b4ac6c7 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -313,18 +313,45 @@ export function startRumSessionManager( endSessionIfSettingsAreDecisive ) - sessionManager.sessionStateUpdateObservable.subscribe(({ previousState, newState }) => { - if (!previousState.forcedReplay && newState.forcedReplay) { - const sessionEntity = sessionManager.findSession() - if (sessionEntity) { - sessionEntity.isReplayForced = true - } + function forceReplay() { + const session = sessionManager.findSession() + if (!session) { + return } - if (!previousState.hasError && newState.hasError) { - const sessionEntity = sessionManager.findSession() - if (sessionEntity) { - sessionEntity.hasError = true - } + const wasForced = session.isReplayForced + session.isReplayForced = true + if (!wasForced) { + lifeCycle.notify(LifeCycleEventType.SESSION_RELEASED, { sessionId: session.id, reason: 'force' }) + } + sessionManager.updateSessionState((state) => (state.id === session.id ? { forcedReplay: '1' } : undefined)) + } + + const sessionStateSubscription = sessionManager.sessionStateUpdateObservable.subscribe(({ newState }) => { + const session = sessionManager.findSession() + if (!session || session.id !== newState.id) { + return + } + const becameForced = !session.isReplayForced && newState.forcedReplay === '1' + const becameErrored = !session.hasError && newState.hasError === '1' + session.isReplayForced ||= becameForced + session.hasError ||= becameErrored + if (becameForced || becameErrored) { + lifeCycle.notify(LifeCycleEventType.SESSION_RELEASED, { + sessionId: session.id, + reason: becameForced ? 'force' : 'error', + }) + } + // A lock retry can be exhausted before a mark reaches storage. The existing poll is the + // next opportunity to reconcile it, and the session identity bounds how long it may live. + if ((session.hasError && newState.hasError !== '1') || (session.isReplayForced && newState.forcedReplay !== '1')) { + sessionManager.updateSessionState((state) => + state.id === session.id + ? { + ...(session.hasError ? { hasError: '1' } : {}), + ...(session.isReplayForced ? { forcedReplay: '1' } : {}), + } + : undefined + ) } }) return { @@ -346,11 +373,12 @@ export function startRumSessionManager( expire: sessionManager.expire, expireObservable: sessionManager.expireObservable, stop: () => { + sessionStateSubscription.unsubscribe() consentSubscription.unsubscribe() remoteConfigSubscription.unsubscribe() drawnHistory.stop() }, - setForcedReplay: () => sessionManager.updateSessionState(() => ({ forcedReplay: '1' })), + setForcedReplay: forceReplay, // FLASHCAT FORK - the escape hatch for "collect this visitor NOW": the host application knows // who needs debugging (its own allow-list, a support flow), the SDK only provides the switch. // A session keeps the decision it was drawn with, so forcing a visitor that was not being @@ -367,7 +395,7 @@ export function startRumSessionManager( session.trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || withholdsReplay(session.trackingType) ) { - sessionManager.updateSessionState(() => ({ forcedReplay: '1' })) + forceReplay() } }, setSessionHasError: (sessionId) => { @@ -377,7 +405,11 @@ export function startRumSessionManager( // through a lock that can defer it by several retries, and until then the withheld buffer // would still read the session as withholding - so an error followed closely by the page or // the session ending would throw away the very buffer the error was meant to release. + const hadError = sessionEntity.hasError sessionEntity.hasError = true + if (!hadError) { + lifeCycle.notify(LifeCycleEventType.SESSION_RELEASED, { sessionId, reason: 'error' }) + } } sessionManager.updateSessionState((state) => (state.id === sessionId ? { hasError: '1' } : undefined)) }, diff --git a/packages/rum-core/src/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts index 5cfe610038..a0057d01f7 100644 --- a/packages/rum-core/src/domain/trackSessionError.spec.ts +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -13,7 +13,7 @@ describe('startSessionErrorTracking', () => { function collect(type: string, source = 'source') { // only error events carry an `error` object; anything else that did would hide a guard that // reads it before checking the type - const event = type === 'error' ? { type, error: { source } } : { type } + const event = type === 'error' ? { type, session: { id: 'session-id' }, error: { source } } : { type } lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, event as unknown as RumEvent & Context) } @@ -25,6 +25,25 @@ describe('startSessionErrorTracking', () => { registerCleanupTask(stop) }) + it('ignores an error from an earlier session without consuming the current session mark', () => { + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { + type: 'error', + session: { id: 'previous-session' }, + error: { source: 'custom' }, + } as unknown as RumEvent & Context) + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + collect('error') + expect(setSessionHasErrorSpy).toHaveBeenCalledOnceWith('session-id') + }) + + it('does not attribute an error without a session id to the current session', () => { + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { + type: 'error', + error: { source: 'custom' }, + } as unknown as RumEvent & Context) + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + it('marks the session on the first collected error', () => { collect('error') diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts index 2fd0e2dcc6..3414744bf9 100644 --- a/packages/rum-core/src/domain/trackSessionError.ts +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -30,7 +30,7 @@ export function startSessionErrorTracking(lifeCycle: LifeCycle, sessionManager: // write also pushes the session's expiry out (`processSessionStoreOperations` expands every // state it persists), which would move where their sessions end. const session = sessionManager.findTrackedSession() - if (!session?.sampledOnErrorReplay) { + if (!session?.sampledOnErrorReplay || event.session?.id !== session.id) { return } hasReportedError = true diff --git a/packages/rum-legacy/src/domain/sessionStore.spec.ts b/packages/rum-legacy/src/domain/sessionStore.spec.ts index fdeddb47a4..9f88cdf96b 100644 --- a/packages/rum-legacy/src/domain/sessionStore.spec.ts +++ b/packages/rum-legacy/src/domain/sessionStore.spec.ts @@ -29,6 +29,31 @@ describe('session store', () => { deleteSessionCookie() }) + for (const [rum, flag, tracked] of [ + ['3', '', true], + ['4', '', false], + ['5', '', false], + ['4', '&hasError=1', true], + ['5', '&hasError=1', true], + ['4', '&forcedReplay=1', true], + ['5', '&forcedReplay=1', true], + ['0', '&hasError=1', false], + ] as const) { + it(`respects the shared tracking decision rum=${rum}${flag}`, () => { + document.cookie = `${SESSION_COOKIE_NAME}=id=shared-session&rum=${rum}${flag}&created=${Date.now()}&expire=${Date.now() + ONE_MINUTE};path=/` + expect(createSessionStore(100).getOrCreateSession().isTracked).toBe(tracked) + expect(toSessionState(readRawCookie()).rum).toBe(rum) + }) + } + + it('does not carry release marks into a renewed legacy session', () => { + document.cookie = `${SESSION_COOKIE_NAME}=id=old-session&rum=5&hasError=1&forcedReplay=1&created=${Date.now() - ONE_MINUTE}&expire=${Date.now() - 1};path=/` + createSessionStore(100).getOrCreateSession() + const stored = toSessionState(readRawCookie()) + expect(stored.hasError).toBeUndefined() + expect(stored.forcedReplay).toBeUndefined() + }) + it('creates a session with a lowercase uuid', () => { const session = createSessionStore(100).getOrCreateSession() diff --git a/packages/rum-legacy/src/domain/sessionStore.ts b/packages/rum-legacy/src/domain/sessionStore.ts index 60a870a3a7..28c49e701e 100644 --- a/packages/rum-legacy/src/domain/sessionStore.ts +++ b/packages/rum-legacy/src/domain/sessionStore.ts @@ -30,6 +30,9 @@ const EXPIRED = '1' const NOT_TRACKED = '0' const TRACKED_WITH_SESSION_REPLAY = '1' const TRACKED_WITHOUT_SESSION_REPLAY = '2' +const TRACKED_WITH_ERROR_SESSION_REPLAY = '3' +const TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY = '4' +const TRACKED_ON_ERROR_WITH_SESSION_REPLAY = '5' /** * How long a session may be reused without touching the cookie again. @@ -133,7 +136,7 @@ export function createSessionStore(sessionSampleRate: number) { } function toSession(state: SessionState): LegacySession { - // Both tracked values count. This build never writes '1' itself, but both builds share one cookie + // Honor collected and released decisions. This build writes only '0'/'2', but shares one cookie // jar per domain, and IE enterprise site lists routinely put some urls of a site in compatibility // mode and others not. Reading a session the modern bundle started as untracked would silence // this one for the rest of that session's lifetime. @@ -144,7 +147,13 @@ function toSession(state: SessionState): LegacySession { } function isTracked(state: SessionState): boolean { - return state.rum === TRACKED_WITHOUT_SESSION_REPLAY || state.rum === TRACKED_WITH_SESSION_REPLAY + return ( + state.rum === TRACKED_WITHOUT_SESSION_REPLAY || + state.rum === TRACKED_WITH_SESSION_REPLAY || + state.rum === TRACKED_WITH_ERROR_SESSION_REPLAY || + ((state.rum === TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY || state.rum === TRACKED_ON_ERROR_WITH_SESSION_REPLAY) && + (state.hasError === '1' || state.forcedReplay === '1')) + ) } /** @@ -193,7 +202,7 @@ function isExpired(state: SessionState, now: number): boolean { // `isExpired` belongs to the modern bundle's vocabulary, not to ours, but it has to be listed here // all the same: carried forward as an unknown field it would mark every session this build writes // as expired, and the modern bundle would start a new one on every page load. -const KNOWN_FIELDS = ['id', 'created', 'expire', 'rum', 'isExpired'] +const KNOWN_FIELDS = ['id', 'created', 'expire', 'rum', 'isExpired', 'hasError', 'forcedReplay'] function serialize(state: SessionState): string { const entries: string[] = [] diff --git a/packages/rum/README.md b/packages/rum/README.md index 11198510c8..e5b56ba897 100644 --- a/packages/rum/README.md +++ b/packages/rum/README.md @@ -32,3 +32,35 @@ flashcatRum.init({ [1]: https://docs.flashcat.cloud/zh/flashduty/rum/introduction [2]: https://www.npmjs.com/package/@flashcatcloud/browser-rum + +## Enabling error session collection across pages + +`sessionReplayOnError` needs the full `browser-rum` bundle. The slim and legacy +bundles do not contain a recorder. `sessionOnError` also requires a bundle with +conditional event buffering; the legacy bundle can only honor a shared session +that has already been released by a compatible modern page. + +Before enabling either option in initialization or remote configuration: + +1. Deploy compatible SDK bundles to every page sharing the session cookie, + including other applications and subdomains when cross-subdomain tracking is + enabled. Keep both error-collection options disabled during this deployment. +2. Account for already-open pages and cached application assets. Publishing a new + SDK does not replace JavaScript in those pages. Require those pages to reload, + or defer enablement until incompatible pages no longer share the session store. +3. Verify navigation and concurrent tabs using the deployed bundles. A session + must keep its identity and conditional decision until an error or explicit + force releases it. Verify that sessions without either trigger upload no + conditional data. +4. Enable the options only after that compatibility check. Before rolling back to + an incompatible bundle, disable conditional collection and end or drain the + existing conditional sessions across the affected pages. Disabling an option + alone does not rewrite every running session's decision. + +Older modern bundles recognize only session tracking values `0`, `1`, and `2`. +They can redraw conditional values `3`, `4`, or `5`, causing unexpected collection +or data loss. The compatible legacy reader recognizes `3` and released `4`/`5`, +but it cannot recover history it never recorded. A browser cannot guarantee +cross-page persistence if its shared store stays locked or becomes unavailable +until the page closes; the SDK retries missing marks through its existing session +poll while that same session remains active. diff --git a/packages/rum/src/domain/replayStats.ts b/packages/rum/src/domain/replayStats.ts index 3a69ce57c1..1dc2370f60 100644 --- a/packages/rum/src/domain/replayStats.ts +++ b/packages/rum/src/domain/replayStats.ts @@ -21,8 +21,8 @@ export function addWroteData(viewId: string, additionalBytesCount: number) { /** * Gives back the segment count {@link addSegment} took, and with it the `index_in_view` the segment - * was holding. Undone in the same phase it was taken - synchronously - because the index is read at - * creation: a segment created before this runs would hold an index the dropped one still occupies. + * was holding. Segment collection serializes encoder operations, so a dropped segment returns its + * reservation after the release decision and before the next segment is created. */ export function removeSegment(viewId: string) { const replayStats = statsPerView?.get(viewId) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index af41d49d3f..dc7d4d1fea 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -383,6 +383,110 @@ describe('startSegmentCollection withholding (error session replay)', () => { }) }) + it('releases a checkout still being encoded without reusing its segment index', async () => { + addRecord({ ...RECORD, type: RecordType.FullSnapshot, data: {} } as BrowserRecord) + worker.processAllMessages() + clock.tick(BUFFER_CHECKOUT_TIME) + reportError() + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: 'error' } as any) + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + const metadata = await Promise.all( + httpRequestSpy.send.calls.allArgs().map(([payload]) => readMetadataFromReplayPayload(payload)) + ) + expect(metadata.map((segment) => segment.index_in_view)).toEqual([0, 1]) + expect(metadata[0]?.has_full_snapshot).toBeTrue() + }) + + it('remembers a release if recording ends before the worker answers', () => { + addRecord(RECORD) + worker.processAllMessages() + clock.tick(BUFFER_CHECKOUT_TIME) + reportError() + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: 'error' } as any) + stopCollection() + releasedSessionId = undefined + worker.processAllMessages() + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + }) + + it('drains records and a stop queued behind a released flush', async () => { + addRecord(RECORD) + worker.processAllMessages() + clock.tick(BUFFER_CHECKOUT_TIME) + addRecord(RECORD) + reportError() + stopCollection() + releasedSessionId = undefined + worker.processAllMessages() + const metadata = await Promise.all( + httpRequestSpy.send.calls.allArgs().map(([payload]) => readMetadataFromReplayPayload(payload)) + ) + expect(metadata.map((segment) => segment.index_in_view)).toEqual([0, 1]) + expect(metadata.map((segment) => segment.records_count)).toEqual([1, 1]) + }) + + it('preserves encoder ordering when a new recording starts before the old flush completes', async () => { + const sharedWorker = new MockWorker() + const sharedEncoder = createDeflateEncoder({} as RumConfiguration, sharedWorker, DeflateEncoderStreamId.REPLAY) + const sent: Array[0]> = [] + let released = false + const request = { send: (payload: Parameters[0]) => sent.push(payload), sendOnExit: noop } + const first = doStartSegmentCollection(lifeCycle, () => CONTEXT, request, sharedEncoder, { + getWithholdingSessionId: () => (released ? undefined : CONTEXT.session.id), + isReleased: () => released, + restartFromFullSnapshot: noop, + }) + first.addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + first.addRecord(RECORD) + released = true + first.stop() + const second = doStartSegmentCollection( + new LifeCycle(), + () => ({ ...CONTEXT, session: { id: 'next-session' }, view: { id: 'next-view' } }), + request, + sharedEncoder, + { + getWithholdingSessionId: () => undefined, + isReleased: () => false, + restartFromFullSnapshot: noop, + } + ) + second.addRecord(RECORD) + second.stop() + sharedWorker.processAllMessages() + const segments = await Promise.all( + sent.map( + async (payload) => + JSON.parse(await ((payload.data as FormData).get('segment') as Blob).text()) as { + session: { id: string } + records: BrowserRecord[] + index_in_view: number + } + ) + ) + expect(segments.map((segment) => segment.session.id)).toEqual([ + CONTEXT.session.id, + CONTEXT.session.id, + 'next-session', + ]) + expect(segments.map((segment) => segment.index_in_view)).toEqual([0, 1, 0]) + expect(segments.map((segment) => segment.records.length)).toEqual([1, 1, 1]) + }) + + it('never releases an unfinished flush for a different session', () => { + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + releasedSessionId = 'different-session' + stopCollection() + worker.processAllMessages() + expect(httpRequestSpy.send).not.toHaveBeenCalled() + }) + it('does not send anything while the session has not reported an error', () => { addRecord(RECORD) clock.tick(SEGMENT_DURATION_LIMIT) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 1319d1102c..7c7ed72d98 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -113,8 +113,6 @@ type SegmentCollectionState = bufferCheckoutTimeoutId: TimeoutId | undefined /** Set when the segment was created while its session was withholding its replay. */ withheldForSessionId: string | undefined - /** The view the segment belongs to, so its index can be given back without waiting on a flush. */ - viewId: string } | { status: SegmentCollectionStatus.Stopped @@ -128,6 +126,10 @@ type SegmentCollectionState = */ type InternalFlushReason = FlushReason | 'buffer_checkout' | 'page_reactivated' +// Recordings can stop and restart while the same encoder is still finishing a segment. +// Serialize at the encoder boundary so their metadata and index reservations cannot overlap. +let encodingQueues: WeakMap void> }> | undefined + export function doStartSegmentCollection( lifeCycle: LifeCycle, getSegmentContext: () => SegmentContext | undefined, @@ -145,15 +147,48 @@ export function doStartSegmentCollection( let droppedBufferCount = 0 let lastBufferRestartAt: RelativeTime | undefined let bufferRestartTimeoutId: TimeoutId | undefined + encodingQueues ||= new WeakMap() + const encodingQueue = encodingQueues.get(encoder) || { flushing: false, operations: [] } + encodingQueues.set(encoder, encodingQueue) + let stopped = false + const withholdingSessionIds = new Set() + const releasedSessionIds = new Set() + + function rememberReleases() { + withholdingSessionIds.forEach((sessionId) => { + if (buffering.isReleased(sessionId)) { + releasedSessionIds.add(sessionId) + } + }) + } + + function runWhenReady(operation: () => void) { + encodingQueue.operations.push(operation) + drainPendingOperations() + } + + function drainPendingOperations() { + while (!encodingQueue.flushing && encodingQueue.operations.length) { + encodingQueue.operations.shift()!() + } + } + + function requestFlush(reason: InternalFlushReason) { + rememberReleases() + if (reason !== 'view_change' && reason !== 'page_reactivated') { + restoreReleasedSnapshot() + } + runWhenReady(() => flushSegment(reason)) + } const { unsubscribe: unsubscribeViewCreated } = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, () => { - flushSegment('view_change') + requestFlush('view_change') }) const { unsubscribe: unsubscribePageMayExit } = lifeCycle.subscribe( LifeCycleEventType.PAGE_MAY_EXIT, (pageMayExitEvent) => { - flushSegment(pageMayExitEvent.reason as FlushReason) + requestFlush(pageMayExitEvent.reason as FlushReason) } ) @@ -161,7 +196,7 @@ export function doStartSegmentCollection( // next one starts fresh with the full snapshot taken by startFullSnapshots on the same event. // Reuses the 'view_change' creation reason to avoid a schema change. const { unsubscribe: unsubscribeReactivated } = lifeCycle.subscribe(LifeCycleEventType.PAGE_REACTIVATED, () => { - flushSegment('page_reactivated') + requestFlush('page_reactivated') }) const { unsubscribe: unsubscribeRumEvent } = lifeCycle.subscribe( @@ -169,7 +204,18 @@ export function doStartSegmentCollection( restoreReleasedSnapshot ) + const { unsubscribe: unsubscribeSessionReleased } = lifeCycle.subscribe( + LifeCycleEventType.SESSION_RELEASED, + ({ sessionId }) => { + if (withholdingSessionIds.has(sessionId)) { + releasedSessionIds.add(sessionId) + } + restoreReleasedSnapshot() + } + ) + function restoreReleasedSnapshot() { + rememberReleases() if (bufferRestartTimeoutId === undefined) { return } @@ -190,15 +236,11 @@ export function doStartSegmentCollection( } function flushSegment(flushReason: InternalFlushReason) { - if (flushReason !== 'view_change' && flushReason !== 'page_reactivated') { - // A release can also arrive through the shared session store without a local error event. - restoreReleasedSnapshot() - } - // Decided once, and against the session that produced the records rather than whatever session - // is current now: a segment must be either dropped or sent as a whole. + // Keep the encoder and index reservation owned by this segment until its asynchronous + // decision settles. Later records retain their emission context while waiting in FIFO order. const withheldForSessionId = state.status === SegmentCollectionStatus.SegmentPending ? state.withheldForSessionId : undefined - const isWithheld = withheldForSessionId !== undefined && !buffering.isReleased(withheldForSessionId) + const isWithheld = withheldForSessionId !== undefined && !releasedSessionIds.has(withheldForSessionId) if (state.status === SegmentCollectionStatus.SegmentPending) { if (isWithheld && flushReason === 'page_reactivated') { @@ -218,21 +260,16 @@ export function doStartSegmentCollection( // An expiring session does not lose it: the session history entry is still open when the // recorder is stopped (`sessionManager.ts` notifies before closing it), so the stop flush // still sees the session as released and sends. Only losing the page outright loses it. - state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) + state.expirationTimeoutId = setTimeout(() => requestFlush('segment_duration_limit'), SEGMENT_DURATION_LIMIT) } return } - if (isWithheld) { - // Given back here, synchronously, rather than in the flush callback below: that callback only - // runs after a round trip to the deflate worker, and a record arriving in between creates a - // segment that reads its `index_in_view` from a count this one still occupies - leaving two - // uploaded segments claiming the same index, and index 0 never uploaded at all. - removeSegment(state.viewId) - } - + encodingQueue.flushing = true state.segment.flush((metadata, encoderResult) => { - if (isWithheld) { + rememberReleases() + if (withheldForSessionId !== undefined && !releasedSessionIds.has(withheldForSessionId)) { + removeSegment(metadata.view.id) // No error was reported, so this buffer is dropped rather than sent. Rolling back what its // records contributed keeps `has_replay` and the counters on view events honest. discardSegmentData(metadata.view.id, encoderResult.rawBytesCount, metadata.records_count) @@ -240,6 +277,8 @@ export function doStartSegmentCollection( // Restarted from here rather than synchronously below, so the fresh full snapshot lands in // the segment that follows this one rather than in the one being thrown away. restartBuffer(flushReason) + encodingQueue.flushing = false + drainPendingOperations() return } @@ -261,6 +300,8 @@ export function doStartSegmentCollection( } else { httpRequest.send(payload) } + encodingQueue.flushing = false + drainPendingOperations() }) clearTimeout(state.expirationTimeoutId) clearTimeout(state.bufferCheckoutTimeoutId) @@ -291,7 +332,7 @@ export function doStartSegmentCollection( if (flushReason !== 'buffer_checkout' && flushReason !== 'segment_bytes_limit') { return } - if (state.status === SegmentCollectionStatus.Stopped) { + if (stopped || state.status === SegmentCollectionStatus.Stopped) { // The flush that got here waited on the deflate worker, and recording was stopped in the // meantime. Re-serializing the document now would cost a full snapshot on a page that asked // to stop, and count records into the replay stats that no segment will ever hold. @@ -311,57 +352,75 @@ export function doStartSegmentCollection( } } - return { - addRecord: (record: BrowserRecord) => { - if (state.status === SegmentCollectionStatus.Stopped) { + function addRecord( + record: BrowserRecord, + context: SegmentContext | undefined, + withheldForSessionId: string | undefined + ) { + if (state.status === SegmentCollectionStatus.Stopped) { + return + } + + if (record.type === RecordType.FullSnapshot) { + // A view change or page reactivation can supply the replacement before the timer does. + clearTimeout(bufferRestartTimeoutId) + bufferRestartTimeoutId = undefined + } + + if (state.status === SegmentCollectionStatus.WaitingForInitialRecord) { + if (!context) { return } - if (record.type === RecordType.FullSnapshot) { - // A view change or page reactivation can supply the replacement before the timer does. - clearTimeout(bufferRestartTimeoutId) - bufferRestartTimeoutId = undefined + state = { + status: SegmentCollectionStatus.SegmentPending, + segment: createSegment({ encoder, context, creationReason: state.nextSegmentCreationReason }), + expirationTimeoutId: setTimeout(() => { + requestFlush('segment_duration_limit') + }, SEGMENT_DURATION_LIMIT), + bufferCheckoutTimeoutId: + withheldForSessionId !== undefined + ? setTimeout(() => { + requestFlush('buffer_checkout') + }, BUFFER_CHECKOUT_TIME) + : undefined, + withheldForSessionId, } + } - if (state.status === SegmentCollectionStatus.WaitingForInitialRecord) { - const context = getSegmentContext() - if (!context) { - return - } - - const withheldForSessionId = buffering.getWithholdingSessionId() - state = { - status: SegmentCollectionStatus.SegmentPending, - segment: createSegment({ encoder, context, creationReason: state.nextSegmentCreationReason }), - expirationTimeoutId: setTimeout(() => { - flushSegment('segment_duration_limit') - }, SEGMENT_DURATION_LIMIT), - bufferCheckoutTimeoutId: - withheldForSessionId !== undefined - ? setTimeout(() => { - flushSegment('buffer_checkout') - }, BUFFER_CHECKOUT_TIME) - : undefined, - withheldForSessionId, - viewId: context.view.id, - } + state.segment.addRecord(record, (encodedBytesCount) => { + if (encodedBytesCount > SEGMENT_BYTES_LIMIT) { + requestFlush('segment_bytes_limit') } + }) + } - state.segment.addRecord(record, (encodedBytesCount) => { - if (encodedBytesCount > SEGMENT_BYTES_LIMIT) { - flushSegment('segment_bytes_limit') - } - }) + return { + addRecord: (record: BrowserRecord) => { + if (stopped) { + return + } + const context = getSegmentContext() + const withheldForSessionId = buffering.getWithholdingSessionId() + if (withheldForSessionId !== undefined) { + withholdingSessionIds.add(withheldForSessionId) + } + rememberReleases() + runWhenReady(() => addRecord(record, context, withheldForSessionId)) }, - stop: () => { - flushSegment('stop') + if (stopped) { + return + } + requestFlush('stop') + stopped = true clearTimeout(bufferRestartTimeoutId) bufferRestartTimeoutId = undefined unsubscribeViewCreated() unsubscribePageMayExit() unsubscribeReactivated() unsubscribeRumEvent() + unsubscribeSessionReleased() }, } }