diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 95fa26abd6..cc4c8c8125 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -292,33 +292,54 @@ describe('rum session manager stub', () => { }) }) - // FLASHCAT FORK - a host that answers for the session id and reports none has no session at all - // right now. Falling back to the placeholder here would attribute this page's Session Replay - // segments — which it uploads itself, bypassing the host — to a fake session shared by every - // application, and reusing the previous id would attribute them to a session that has ended. + // FLASHCAT FORK - a host that answers for the session id and reports none has no session right + // now. What this page produces has to be split: events reach the intake through the host, which + // overrides their session id, so they keep flowing — a host may depend on them to know the user + // is still around, and `fc-sdk-electron` renews its session from the click actions reported + // here. Session Replay segments bypass the host entirely, so nothing overrides the placeholder + // they would carry, and it is a constant shared by every application; recording stops instead. describe('when the host reports no session', () => { - it('should report no tracked session', () => { + it('should keep reporting a tracked session, so events still reach the host', () => { mockEventBridge({ sessionId: '', anonymousId: '' }) - expect(startStub().findTrackedSession()).toBeUndefined() + expect(startStub().findTrackedSession()).toBeDefined() }) - it('should not fall back to the placeholder session id', () => { + it('should stop Session Replay, which the host does not get a chance to attribute', () => { mockEventBridge({ sessionId: '' }) - expect(startStub().findTrackedSession()?.id).not.toBe(STUB_SESSION_ID) + expect(startStub().findTrackedSession()!.sessionReplay).toBe(SessionReplayState.OFF) }) - it('should report a tracked session again once the host renews it', () => { - let sessionId = '' + it('should name the session with the placeholder the host overrides', () => { + mockEventBridge({ sessionId: '' }) + + expect(startStub().findTrackedSession()!.id).toBe(STUB_SESSION_ID) + }) + + it('should never report the id the host held before, which belongs to a session that ended', () => { + let sessionId = 'ended-session-id' const eventBridge = mockEventBridge({ sessionId: '' }) eventBridge.getSessionId = () => sessionId const sessionManager = startStub() + expect(sessionManager.findTrackedSession()!.id).toBe('ended-session-id') + + sessionId = '' - expect(sessionManager.findTrackedSession()).toBeUndefined() + expect(sessionManager.findTrackedSession()!.id).not.toBe('ended-session-id') + }) + + it('should record again once the host renews its session', () => { + let sessionId = '' + const eventBridge = mockEventBridge({ sessionId: '' }) + eventBridge.getSessionId = () => sessionId + const sessionManager = startStub() + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.OFF) sessionId = 'renewed-session-id' + expect(sessionManager.findTrackedSession()!.id).toBe('renewed-session-id') + 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 7ebf9d9f7d..894cdb0898 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -100,13 +100,15 @@ export function startRumSessionManager( } /** - * Session id used when the host application does not answer for one, because it was built against - * an SDK that predates `getSessionId()`. Such a host is expected to override the session id of the - * events it forwards, so the placeholder never reaches the intake for RUM events. + * Session id used whenever the host application cannot name the session this page is in — either + * because it was built against an SDK that predates `getSessionId()`, or because it has no session + * at this moment. The host overrides the session id of every event it forwards, so this never + * reaches the intake through the bridge. * - * It would reach the intake for Session Replay segments, which this page uploads directly, and the - * placeholder is a constant shared by every application — so a host that DOES answer for the - * session id must never end up on it. See `startRumSessionManagerStub`. + * It would reach the intake for Session Replay segments, which this page uploads directly, and it + * is a constant shared by every application built on this SDK. Nothing may therefore be recorded + * while this is the session id — see `startRumSessionManagerStub`, which pairs it with + * `SessionReplayState.OFF`. */ export const STUB_SESSION_ID = '00000000-aaaa-0000-aaaa-000000000000' @@ -182,18 +184,28 @@ export function startRumSessionManagerStub( // Read from the bridge on each call rather than from `lastSeenSessionId`: the poll above only // exists to spot transitions, and an event assembled between two polls must still carry the id // the host holds right now. - findTrackedSession: (): RumSession | undefined => { - const sessionId = readHostSessionId() - if (sessionId === NO_HOST_SESSION) { - // The host owns the session and currently has none. There is nothing to attribute this - // page's data to, so it has no tracked session either — falling back to the placeholder - // would pile Session Replay segments onto a fake session shared across applications, and - // reusing the id the host had a moment ago would pile them onto a session that ended. - return - } + findTrackedSession: (): RumSession => { + const hostSessionId = readHostSessionId() + const hasHostSession = hostSessionId !== NO_HOST_SESSION + + // While the host has no session, the two kinds of data this page produces carry very + // different risk, so they are treated differently rather than both being dropped. + // + // Events go to the host, which overrides their session id before forwarding them — the + // placeholder never reaches the intake through the bridge, so they keep flowing. They have + // to: a host may well depend on them to know the user is still there. `fc-sdk-electron` + // renews its session from the click actions this page reports, so a page that stops + // reporting during the gap leaves the host with nothing to renew from, and the session that + // was only paused never comes back. + // + // Session Replay segments are uploaded by this page directly, so nothing overrides them. + // There is no session to attribute them to, and the placeholder is a constant every + // application built on this SDK shares — so recording stops instead, and `SESSION_EXPIRED` + // (notified by the poll above) makes the recorder flush what it holds rather than keep a + // segment open across the gap. return { - id: sessionId ?? STUB_SESSION_ID, - sessionReplay, + id: hasHostSession && hostSessionId ? hostSessionId : STUB_SESSION_ID, + sessionReplay: hasHostSession ? sessionReplay : SessionReplayState.OFF, anonymousId: bridge?.getAnonymousId(), } },