From a7e70f2282e8d4e7c54811657a26bb22ce57f932 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 10 Jan 2024 22:02:37 +0000 Subject: [PATCH 1/7] fix: set the session id as soon as it changes --- src/extensions/replay/sessionrecording.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index d3c71eeab..4714f37eb 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -423,15 +423,15 @@ export class SessionRecording { const sessionIdChanged = this.sessionId !== sessionId const windowIdChanged = this.windowId !== windowId + this.windowId = windowId + this.sessionId = sessionId + if ( [FULL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE].indexOf(event.type) === -1 && (windowIdChanged || sessionIdChanged) ) { this._tryTakeFullSnapshot() } - - this.windowId = windowId - this.sessionId = sessionId } private _tryRRwebMethod(rrwebMethod: () => void): boolean { From f593c9429b5dffa0ba567c3b08e97461e20caf92 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 11 Jan 2024 10:41:12 +0000 Subject: [PATCH 2/7] un-nest target tests --- .../replay/sessionrecording.test.ts | 132 +++++++++--------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 1409004fc..f93777ca8 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -969,78 +969,78 @@ describe('SessionRecording', () => { expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) }) }) + }) + }) - describe('idle timeouts', () => { - it("enters idle state if the activity is non-user generated and there's no activity for 5 seconds", () => { - sessionRecording.startRecordingIfEnabled() - const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] - expect(lastActivityTimestamp).toBeGreaterThan(0) - - expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) + describe('idle timeouts', () => { + it("enters idle state if the activity is non-user generated and there's no activity for 5 seconds", () => { + sessionRecording.startRecordingIfEnabled() + const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] + expect(lastActivityTimestamp).toBeGreaterThan(0) - _emit({ - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 1, - }, - timestamp: lastActivityTimestamp + 100, - }) - expect(sessionRecording['isIdle']).toEqual(false) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) - expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - - _emit({ - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 0, - }, - timestamp: lastActivityTimestamp + 200, - }) - expect(sessionRecording['isIdle']).toEqual(false) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) - expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + _emit({ + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 1, + }, + timestamp: lastActivityTimestamp + 100, + }) + expect(sessionRecording['isIdle']).toEqual(false) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) - // this triggers idle state and isn't a user interaction so does not take a full snapshot - _emit({ - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 0, - }, - timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000, - }) - expect(sessionRecording['isIdle']).toEqual(true) - expect(_addCustomEvent).toHaveBeenCalledWith('sessionIdle', { - reason: 'user inactivity', - threshold: 300000, - timeSinceLastActive: 300900, - }) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) - expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - // this triggers idle state _and_ is a user interaction, so we take a full snapshot - _emit({ - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 1, - }, - timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000, - }) - expect(sessionRecording['isIdle']).toEqual(false) - expect(_addCustomEvent).toHaveBeenCalledWith('sessionNoLongerIdle', { - reason: 'user activity', - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - }) - expect(sessionRecording['_lastActivityTimestamp']).toEqual( - lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 - ) - expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) - }) + _emit({ + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 0, + }, + timestamp: lastActivityTimestamp + 200, + }) + expect(sessionRecording['isIdle']).toEqual(false) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + + // this triggers idle state and isn't a user interaction so does not take a full snapshot + _emit({ + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 0, + }, + timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000, + }) + expect(sessionRecording['isIdle']).toEqual(true) + expect(_addCustomEvent).toHaveBeenCalledWith('sessionIdle', { + reason: 'user inactivity', + threshold: 300000, + timeSinceLastActive: 300900, }) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + + // this triggers idle state _and_ is a user interaction, so we take a full snapshot + _emit({ + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 1, + }, + timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000, + }) + expect(sessionRecording['isIdle']).toEqual(false) + expect(_addCustomEvent).toHaveBeenCalledWith('sessionNoLongerIdle', { + reason: 'user activity', + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + }) + expect(sessionRecording['_lastActivityTimestamp']).toEqual( + lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 + ) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) }) }) From e0b378fcc6bd52a8f0ad50d4ca9c128458ad153d Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 11 Jan 2024 12:42:45 +0000 Subject: [PATCH 3/7] a little duplication --- .../replay/sessionrecording.test.ts | 209 +++++++++++++++++- src/extensions/replay/sessionrecording.ts | 8 +- 2 files changed, 204 insertions(+), 13 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index f93777ca8..d47bc6b71 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -973,47 +973,75 @@ describe('SessionRecording', () => { }) describe('idle timeouts', () => { - it("enters idle state if the activity is non-user generated and there's no activity for 5 seconds", () => { + it("enters idle state within one session if the activity is non-user generated and there's no activity for (RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) 5 minutes", () => { sessionRecording.startRecordingIfEnabled() const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] expect(lastActivityTimestamp).toBeGreaterThan(0) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) - _emit({ + // the buffer starts out empty + expect(sessionRecording['buffer']).toEqual({ + data: [], + sessionId: null, + size: 0, + windowId: null, + }) + + const firstSnapshotEvent = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 1, }, timestamp: lastActivityTimestamp + 100, - }) + } + _emit(firstSnapshotEvent) expect(sessionRecording['isIdle']).toEqual(false) expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - _emit({ + const firstSessionId = sessionRecording['sessionId'] + // after the first emit the buffer has been initialised but not flushed + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent], + sessionId: firstSessionId, + size: 68, + windowId: expect.any(String), + }) + + const secondSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 0, }, timestamp: lastActivityTimestamp + 200, - }) + } + _emit(secondSnapshot) expect(sessionRecording['isIdle']).toEqual(false) expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + // the second snapshot remains buffered in memory + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent, secondSnapshot], + sessionId: firstSessionId, + size: 136, + windowId: expect.any(String), + }) + // this triggers idle state and isn't a user interaction so does not take a full snapshot - _emit({ + const thirdSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 0, }, timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000, - }) + } + _emit(thirdSnapshot) expect(sessionRecording['isIdle']).toEqual(true) expect(_addCustomEvent).toHaveBeenCalledWith('sessionIdle', { reason: 'user inactivity', @@ -1023,15 +1051,25 @@ describe('SessionRecording', () => { expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - // this triggers idle state _and_ is a user interaction, so we take a full snapshot - _emit({ + // the third snapshot is dropped since it switches the session to idle + // the custom event doesn't show here since there's not a real rrweb to emit it + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent, secondSnapshot], + sessionId: firstSessionId, + size: 136, + windowId: expect.any(String), + }) + + // this triggers exit from idle state _and_ is a user interaction, so we take a full snapshot + const fourthSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 1, }, timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000, - }) + } + _emit(fourthSnapshot) expect(sessionRecording['isIdle']).toEqual(false) expect(_addCustomEvent).toHaveBeenCalledWith('sessionNoLongerIdle', { reason: 'user activity', @@ -1041,6 +1079,157 @@ describe('SessionRecording', () => { lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 ) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + + // this interaction is considered part of the existing session because a new session id has not been generated + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent, secondSnapshot, fourthSnapshot], + sessionId: firstSessionId, + size: 204, + windowId: expect.any(String), + }) + + // because not enough time passed while idle we still have the same session id at the end of this sequence + const endingSessionId = sessionRecording['sessionId'] + expect(endingSessionId).toEqual(firstSessionId) + }) + + it('rotates session if idle for (MAX_SESSION_IDLE_TIMEOUT) 30 minutes', () => { + sessionRecording.startRecordingIfEnabled() + const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] + expect(lastActivityTimestamp).toBeGreaterThan(0) + + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) + + // the buffer starts out empty + expect(sessionRecording['buffer']).toEqual({ + data: [], + sessionId: null, + size: 0, + windowId: null, + }) + + const firstActivityTimestamp = lastActivityTimestamp + 100 + const firstSnapshotEvent = { + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 1, + }, + timestamp: firstActivityTimestamp, + } + _emit(firstSnapshotEvent) + expect(sessionRecording['isIdle']).toEqual(false) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) + + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + + const firstSessionId = sessionRecording['sessionId'] + // after the first emit the buffer has been initialised but not flushed + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent], + sessionId: firstSessionId, + size: 68, + windowId: expect.any(String), + }) + + // the session id generator returns a fixed value but we want it to rotate in part of this test + sessionIdGeneratorMock.mockClear() + const rotatedSessionId = 'rotated-session-id' + sessionIdGeneratorMock.mockImplementation(() => rotatedSessionId) + + const secondActivityTimestamp = lastActivityTimestamp + 200 + const secondSnapshot = { + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 0, + }, + timestamp: secondActivityTimestamp, + } + _emit(secondSnapshot) + expect(sessionRecording['isIdle']).toEqual(false) + // TODO why is this not the second activity timestamp 🙃 + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(sessionIdGeneratorMock).not.toHaveBeenCalled() + + // the second snapshot remains buffered in memory + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent, secondSnapshot], + sessionId: firstSessionId, + size: 136, + windowId: expect.any(String), + }) + + // this triggers idle state and isn't a user interaction so does not take a full snapshot + const thirdActivityTimestamp = sessionManager['_sessionTimeoutMs'] + lastActivityTimestamp + 1 + const thirdSnapshot = { + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 0, + }, + // MAX_SESSION_IDLE_TIMEOUT is 30 minutes in _SECONDS_ + timestamp: thirdActivityTimestamp, + } + _addCustomEvent.mockClear() + _emit(thirdSnapshot) + expect(sessionRecording['isIdle']).toEqual(true) + expect(_addCustomEvent.mock.calls).toEqual([ + [ + 'sessionIdle', + { + reason: 'user inactivity', + threshold: 300000, + timeSinceLastActive: 1799901, + }, + ], + ]) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) + expect(sessionIdGeneratorMock).not.toHaveBeenCalled() + + // the third snapshot is dropped since it switches the session to idle + // the custom event doesn't show here since there's not a real rrweb to emit it + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent, secondSnapshot], + sessionId: firstSessionId, + size: 136, + windowId: expect.any(String), + }) + + // this triggers exit from idle state _and_ is a user interaction, so we take a full snapshot + const fourthActivityTimestamp = sessionManager['_sessionTimeoutMs'] + 1000 + const fourthSnapshot = { + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 1, + }, + timestamp: fourthActivityTimestamp, + } + _emit(fourthSnapshot) + expect(sessionRecording['isIdle']).toEqual(false) + expect(_addCustomEvent).toHaveBeenCalledWith('sessionNoLongerIdle', { + reason: 'user activity', + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + }) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(fourthActivityTimestamp) + expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) + expect(sessionIdGeneratorMock).toHaveBeenCalled() + + // this interaction is considered part of the existing session + // even though it's the "last" activity that causes the session to rotate + expect(sessionRecording['buffer']).toEqual({ + data: [firstSnapshotEvent, secondSnapshot, fourthSnapshot], + sessionId: firstSessionId, + size: 198, + windowId: expect.any(String), + }) + + // but enough time has passed while idle the session id should have changed + const endingSessionId = sessionRecording['sessionId'] + expect(endingSessionId).toEqual(rotatedSessionId) }) }) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 4714f37eb..3b0f8a75d 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -397,6 +397,7 @@ export class SessionRecording { } } + let returningFromIdle = false if (isUserInteraction) { this._lastActivityTimestamp = event.timestamp if (this.isIdle) { @@ -406,7 +407,7 @@ export class SessionRecording { reason: 'user activity', type: event.type, }) - this._tryTakeFullSnapshot() + returningFromIdle = true } } @@ -427,8 +428,9 @@ export class SessionRecording { this.sessionId = sessionId if ( - [FULL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE].indexOf(event.type) === -1 && - (windowIdChanged || sessionIdChanged) + returningFromIdle || + ([FULL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE].indexOf(event.type) === -1 && + (windowIdChanged || sessionIdChanged)) ) { this._tryTakeFullSnapshot() } From 8eec4275d05eac9f81b55c1079c548ec066fe064 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 11 Jan 2024 13:57:15 +0000 Subject: [PATCH 4/7] get the rotating test running more realistically: --- .../replay/sessionrecording.test.ts | 153 +++++++++++++----- 1 file changed, 111 insertions(+), 42 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index d47bc6b71..a4744d6d1 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -9,7 +9,11 @@ import { SESSION_RECORDING_RECORDER_VERSION_SERVER_SIDE, } from '../../../constants' import { SessionIdManager } from '../../../sessionid' -import { INCREMENTAL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE } from '../../../extensions/replay/sessionrecording-utils' +import { + FULL_SNAPSHOT_EVENT_TYPE, + INCREMENTAL_SNAPSHOT_EVENT_TYPE, + META_EVENT_TYPE, +} from '../../../extensions/replay/sessionrecording-utils' import { PostHog } from '../../../posthog-core' import { DecideResponse, PostHogConfig, Property, SessionIdChangedCallback } from '../../../types' import { uuidv7 } from '../../../uuidv7' @@ -29,6 +33,19 @@ jest.mock('../../../utils', () => ({ })) jest.mock('../../../config', () => ({ LIB_VERSION: 'v0.0.1' })) +const EMPTY_BUFFER = { + data: [], + sessionId: null, + size: 0, + windowId: null, +} + +const createFullSnapshot = (event = {}) => ({ + type: FULL_SNAPSHOT_EVENT_TYPE, + data: {}, + ...event, +}) + const createIncrementalSnapshot = (event = {}) => ({ type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { @@ -58,7 +75,10 @@ describe('SessionRecording', () => { _emit = emit return () => {} }) - assignableWindow.rrwebRecord.takeFullSnapshot = jest.fn() + assignableWindow.rrwebRecord.takeFullSnapshot = jest.fn(() => { + // we pretend to be rrweb and call emit + _emit(createFullSnapshot()) + }) assignableWindow.rrwebRecord.addCustomEvent = _addCustomEvent assignableWindow.rrwebConsoleRecord = { @@ -229,9 +249,26 @@ describe('SessionRecording', () => { sessionRecording.startRecordingIfEnabled() expect(loadScript).toHaveBeenCalled() expect(sessionRecording['status']).toBe('buffering') + expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) _emit(createIncrementalSnapshot({ data: { source: 1 } })) - expect(sessionRecording['buffer']?.data.length).toEqual(1) + expect(sessionRecording['buffer']).toEqual({ + data: [ + { + data: {}, + type: 2, + }, + { + data: { + source: 1, + }, + type: 3, + }, + ], + sessionId: sessionId, + size: 50, + windowId: 'windowId', + }) sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: undefined })) expect(sessionRecording['status']).toBe('disabled') @@ -427,6 +464,7 @@ describe('SessionRecording', () => { expect(sessionRecording['buffer']).toEqual({ data: [ + createFullSnapshot(), { data: { source: 1, @@ -436,7 +474,7 @@ describe('SessionRecording', () => { ], // session id and window id are not null 🚀 sessionId: sessionId, - size: 30, + size: 50, windowId: 'windowId', }) @@ -454,8 +492,9 @@ describe('SessionRecording', () => { expect(posthog.capture).toHaveBeenCalledWith( '$snapshot', { - $snapshot_bytes: 60, + $snapshot_bytes: 80, $snapshot_data: [ + createFullSnapshot(), { type: 3, data: { source: 1 } }, { type: 3, data: { source: 2 } }, ], @@ -493,8 +532,9 @@ describe('SessionRecording', () => { { $session_id: sessionId, $window_id: 'windowId', - $snapshot_bytes: 60, + $snapshot_bytes: 80, $snapshot_data: [ + createFullSnapshot(), { type: 3, data: { source: 1 } }, { type: 3, data: { source: 2 } }, ], @@ -521,7 +561,7 @@ describe('SessionRecording', () => { _emit(createIncrementalSnapshot({ data: { source: 1, payload: 2 } })) expect(posthog.capture).not.toHaveBeenCalled() - expect(sessionRecording['buffer']).toMatchObject({ size: 755101 }) + expect(sessionRecording['buffer']).toMatchObject({ size: 755121 }) // Another big event means the old data will be flushed _emit(createIncrementalSnapshot({ data: { source: 1, payload: bigData } })) @@ -537,21 +577,22 @@ describe('SessionRecording', () => { const bigData = 'a'.repeat(RECORDING_MAX_EVENT_SIZE * 0.8) _emit(createIncrementalSnapshot({ data: { source: 1, payload: bigData } })) - expect(sessionRecording['buffer']).toMatchObject({ size: 755017 }) // the size of the big data event + expect(sessionRecording['buffer']).toMatchObject({ size: 755037 }) // the size of the big data event + expect(sessionRecording['buffer']?.data.length).toEqual(2) // full snapshot and a big event _emit(createIncrementalSnapshot({ data: { source: 1, payload: 1 } })) _emit(createIncrementalSnapshot({ data: { source: 1, payload: 2 } })) expect(posthog.capture).not.toHaveBeenCalled() - expect(sessionRecording['buffer']).toMatchObject({ size: 755101 }) + expect(sessionRecording['buffer']).toMatchObject({ size: 755121 }) // Another big event means the old data will be flushed _emit(createIncrementalSnapshot({ data: { source: 1, payload: bigData } })) // but the recording is still buffering expect(sessionRecording['status']).toBe('buffering') expect(posthog.capture).not.toHaveBeenCalled() - expect(sessionRecording['buffer']?.data.length).toEqual(4) // The new event - expect(sessionRecording['buffer']).toMatchObject({ size: 755017 + 755101 }) // the size of the big data event + expect(sessionRecording['buffer']?.data.length).toEqual(5) // + the new event + expect(sessionRecording['buffer']).toMatchObject({ size: 755037 + 755101 }) // the size of the big data event }) it('flushes buffer if the session_id changes', () => { @@ -564,7 +605,10 @@ describe('SessionRecording', () => { expect(posthog.capture).not.toHaveBeenCalled() expect(sessionRecording['buffer']?.sessionId).not.toEqual(null) - expect(sessionRecording['buffer']?.data).toEqual([{ data: { source: 1 }, emit: 1, type: 3 }]) + expect(sessionRecording['buffer']?.data).toEqual([ + createFullSnapshot(), + { data: { source: 1 }, emit: 1, type: 3 }, + ]) // Not exactly right but easier to test than rotating the session id // this simulates as the session id changing _after_ it has initially been set @@ -577,8 +621,8 @@ describe('SessionRecording', () => { { $session_id: 'otherSessionId', $window_id: 'windowId', - $snapshot_data: [{ data: { source: 1 }, emit: 1, type: 3 }], - $snapshot_bytes: 39, + $snapshot_data: [createFullSnapshot(), { data: { source: 1 }, emit: 1, type: 3 }], + $snapshot_bytes: 59, }, { method: 'POST', @@ -975,6 +1019,8 @@ describe('SessionRecording', () => { describe('idle timeouts', () => { it("enters idle state within one session if the activity is non-user generated and there's no activity for (RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) 5 minutes", () => { sessionRecording.startRecordingIfEnabled() + sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + expect(sessionRecording['status']).toEqual('active') const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] expect(lastActivityTimestamp).toBeGreaterThan(0) @@ -1005,9 +1051,9 @@ describe('SessionRecording', () => { const firstSessionId = sessionRecording['sessionId'] // after the first emit the buffer has been initialised but not flushed expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent], + data: [createFullSnapshot(), firstSnapshotEvent], sessionId: firstSessionId, - size: 68, + size: 88, windowId: expect.any(String), }) @@ -1026,9 +1072,9 @@ describe('SessionRecording', () => { // the second snapshot remains buffered in memory expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent, secondSnapshot], + data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], sessionId: firstSessionId, - size: 136, + size: 156, windowId: expect.any(String), }) @@ -1054,9 +1100,9 @@ describe('SessionRecording', () => { // the third snapshot is dropped since it switches the session to idle // the custom event doesn't show here since there's not a real rrweb to emit it expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent, secondSnapshot], + data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], sessionId: firstSessionId, - size: 136, + size: 156, windowId: expect.any(String), }) @@ -1080,11 +1126,12 @@ describe('SessionRecording', () => { ) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) - // this interaction is considered part of the existing session because a new session id has not been generated + // the fourth snapshot should trigger a flush because the session id has changed... expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent, secondSnapshot, fourthSnapshot], + // as we return from idle we will capture a full snapshot _before_ the fourth snapshot + data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot, createFullSnapshot(), fourthSnapshot], sessionId: firstSessionId, - size: 204, + size: 244, windowId: expect.any(String), }) @@ -1095,6 +1142,9 @@ describe('SessionRecording', () => { it('rotates session if idle for (MAX_SESSION_IDLE_TIMEOUT) 30 minutes', () => { sessionRecording.startRecordingIfEnabled() + sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + expect(sessionRecording['status']).toEqual('active') + const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] expect(lastActivityTimestamp).toBeGreaterThan(0) @@ -1126,9 +1176,9 @@ describe('SessionRecording', () => { const firstSessionId = sessionRecording['sessionId'] // after the first emit the buffer has been initialised but not flushed expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent], + data: [createFullSnapshot(), firstSnapshotEvent], sessionId: firstSessionId, - size: 68, + size: 88, windowId: expect.any(String), }) @@ -1155,9 +1205,9 @@ describe('SessionRecording', () => { // the second snapshot remains buffered in memory expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent, secondSnapshot], + data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], sessionId: firstSessionId, - size: 136, + size: 156, windowId: expect.any(String), }) @@ -1192,12 +1242,15 @@ describe('SessionRecording', () => { // the third snapshot is dropped since it switches the session to idle // the custom event doesn't show here since there's not a real rrweb to emit it expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent, secondSnapshot], + data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], sessionId: firstSessionId, - size: 136, + size: 156, windowId: expect.any(String), }) + // at this point nothing has caused the session to be flushed to the backend + expect(posthog.capture).not.toHaveBeenCalled() + // this triggers exit from idle state _and_ is a user interaction, so we take a full snapshot const fourthActivityTimestamp = sessionManager['_sessionTimeoutMs'] + 1000 const fourthSnapshot = { @@ -1218,18 +1271,34 @@ describe('SessionRecording', () => { expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) expect(sessionIdGeneratorMock).toHaveBeenCalled() - // this interaction is considered part of the existing session - // even though it's the "last" activity that causes the session to rotate + // the fourth snapshot causes the session id to change + const endingSessionId = sessionRecording['sessionId'] + expect(endingSessionId).toEqual(rotatedSessionId) + + // the buffer is flushed, and a full snapshot is taken + expect(posthog.capture).toHaveBeenCalledWith( + '$snapshot', + { + $snapshot_data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], + $session_id: firstSessionId, + $snapshot_bytes: 156, + $window_id: expect.any(String), + }, + { + _batchKey: 'recordings', + _metrics: { rrweb_full_snapshot: false }, + _noTruncate: true, + endpoint: '/s/', + method: 'POST', + transport: 'XHR', + } + ) expect(sessionRecording['buffer']).toEqual({ - data: [firstSnapshotEvent, secondSnapshot, fourthSnapshot], - sessionId: firstSessionId, - size: 198, + data: [createFullSnapshot(), fourthSnapshot], + sessionId: rotatedSessionId, + size: 82, windowId: expect.any(String), }) - - // but enough time has passed while idle the session id should have changed - const endingSessionId = sessionRecording['sessionId'] - expect(endingSessionId).toEqual(rotatedSessionId) }) }) @@ -1308,7 +1377,7 @@ describe('SessionRecording', () => { expect(sessionRecording['sessionDuration']).toBe(100) expect(sessionRecording['_minimumDuration']).toBe(1500) - expect(sessionRecording['buffer']?.data.length).toBe(1) + expect(sessionRecording['buffer']?.data.length).toBe(2) // full snapshot and the emitted incremental event // call the private method to avoid waiting for the timer sessionRecording['_flushBuffer']() @@ -1333,7 +1402,7 @@ describe('SessionRecording', () => { expect(sessionRecording['sessionDuration']).toBe(-1000) expect(sessionRecording['_minimumDuration']).toBe(1500) - expect(sessionRecording['buffer']?.data.length).toBe(1) + expect(sessionRecording['buffer']?.data.length).toBe(2) // full snapshot and the emitted incremental event // call the private method to avoid waiting for the timer sessionRecording['_flushBuffer']() @@ -1353,7 +1422,7 @@ describe('SessionRecording', () => { expect(sessionRecording['sessionDuration']).toBe(100) expect(sessionRecording['_minimumDuration']).toBe(1500) - expect(sessionRecording['buffer']?.data.length).toBe(1) + expect(sessionRecording['buffer']?.data.length).toBe(2) // full snapshot and the emitted incremental event // call the private method to avoid waiting for the timer sessionRecording['_flushBuffer']() @@ -1361,7 +1430,7 @@ describe('SessionRecording', () => { _emit(createIncrementalSnapshot({ data: { source: 1 }, timestamp: sessionStartTimestamp + 1501 })) - expect(sessionRecording['buffer']?.data.length).toBe(2) + expect(sessionRecording['buffer']?.data.length).toBe(3) // full snapshot and two emitted incremental events // call the private method to avoid waiting for the timer sessionRecording['_flushBuffer']() From 26371c220319ee50bf52b90316e85b6d79f4fa16 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 11 Jan 2024 14:03:46 +0000 Subject: [PATCH 5/7] remove a little duplication --- .../replay/sessionrecording.test.ts | 48 +++++++++---------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index a4744d6d1..c85d4894a 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -1017,12 +1017,15 @@ describe('SessionRecording', () => { }) describe('idle timeouts', () => { - it("enters idle state within one session if the activity is non-user generated and there's no activity for (RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) 5 minutes", () => { + let startingTimestamp = -1 + + beforeEach(() => { sessionRecording.startRecordingIfEnabled() sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) expect(sessionRecording['status']).toEqual('active') - const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] - expect(lastActivityTimestamp).toBeGreaterThan(0) + + startingTimestamp = sessionRecording['_lastActivityTimestamp'] + expect(startingTimestamp).toBeGreaterThan(0) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) @@ -1033,18 +1036,21 @@ describe('SessionRecording', () => { size: 0, windowId: null, }) + }) + it("enters idle state within one session if the activity is non-user generated and there's no activity for (RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) 5 minutes", () => { + const firstActivityTimestamp = startingTimestamp + 100 const firstSnapshotEvent = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 1, }, - timestamp: lastActivityTimestamp + 100, + timestamp: firstActivityTimestamp, } _emit(firstSnapshotEvent) expect(sessionRecording['isIdle']).toEqual(false) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) @@ -1057,17 +1063,18 @@ describe('SessionRecording', () => { windowId: expect.any(String), }) + const secondActivityTimestamp = startingTimestamp + 200 const secondSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 0, }, - timestamp: lastActivityTimestamp + 200, + timestamp: secondActivityTimestamp, } _emit(secondSnapshot) expect(sessionRecording['isIdle']).toEqual(false) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) // the second snapshot remains buffered in memory @@ -1079,13 +1086,14 @@ describe('SessionRecording', () => { }) // this triggers idle state and isn't a user interaction so does not take a full snapshot + const thirdActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000 const thirdSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 0, }, - timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000, + timestamp: thirdActivityTimestamp, } _emit(thirdSnapshot) expect(sessionRecording['isIdle']).toEqual(true) @@ -1094,7 +1102,7 @@ describe('SessionRecording', () => { threshold: 300000, timeSinceLastActive: 300900, }) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(lastActivityTimestamp + 100) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) // the third snapshot is dropped since it switches the session to idle @@ -1107,13 +1115,14 @@ describe('SessionRecording', () => { }) // this triggers exit from idle state _and_ is a user interaction, so we take a full snapshot + const fourthActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 const fourthSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, data: { source: 1, }, - timestamp: lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000, + timestamp: fourthActivityTimestamp, } _emit(fourthSnapshot) expect(sessionRecording['isIdle']).toEqual(false) @@ -1121,9 +1130,7 @@ describe('SessionRecording', () => { reason: 'user activity', type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, }) - expect(sessionRecording['_lastActivityTimestamp']).toEqual( - lastActivityTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 - ) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(fourthActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) // the fourth snapshot should trigger a flush because the session id has changed... @@ -1141,15 +1148,6 @@ describe('SessionRecording', () => { }) it('rotates session if idle for (MAX_SESSION_IDLE_TIMEOUT) 30 minutes', () => { - sessionRecording.startRecordingIfEnabled() - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) - expect(sessionRecording['status']).toEqual('active') - - const lastActivityTimestamp = sessionRecording['_lastActivityTimestamp'] - expect(lastActivityTimestamp).toBeGreaterThan(0) - - expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(0) - // the buffer starts out empty expect(sessionRecording['buffer']).toEqual({ data: [], @@ -1158,7 +1156,7 @@ describe('SessionRecording', () => { windowId: null, }) - const firstActivityTimestamp = lastActivityTimestamp + 100 + const firstActivityTimestamp = startingTimestamp + 100 const firstSnapshotEvent = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, @@ -1187,7 +1185,7 @@ describe('SessionRecording', () => { const rotatedSessionId = 'rotated-session-id' sessionIdGeneratorMock.mockImplementation(() => rotatedSessionId) - const secondActivityTimestamp = lastActivityTimestamp + 200 + const secondActivityTimestamp = startingTimestamp + 200 const secondSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, @@ -1212,7 +1210,7 @@ describe('SessionRecording', () => { }) // this triggers idle state and isn't a user interaction so does not take a full snapshot - const thirdActivityTimestamp = sessionManager['_sessionTimeoutMs'] + lastActivityTimestamp + 1 + const thirdActivityTimestamp = sessionManager['_sessionTimeoutMs'] + startingTimestamp + 1 const thirdSnapshot = { event: 123, type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, From 5f51c06401bfead0aa1e9fd7b3b63250f8ad3207 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 11 Jan 2024 14:26:48 +0000 Subject: [PATCH 6/7] make the duplication between tests more obvious --- .../replay/sessionrecording.test.ts | 206 ++++++++---------- 1 file changed, 86 insertions(+), 120 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index c85d4894a..e644485fe 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -1019,6 +1019,35 @@ describe('SessionRecording', () => { describe('idle timeouts', () => { let startingTimestamp = -1 + function emitInactiveEvent(activityTimestamp: number, expectIdle: boolean = false) { + const snapshotEvent = { + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 0, + }, + timestamp: activityTimestamp, + } + _emit(snapshotEvent) + expect(sessionRecording['isIdle']).toEqual(expectIdle) + return snapshotEvent + } + + function emitActiveEvent(activityTimestamp: number) { + const snapshotEvent = { + event: 123, + type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, + data: { + source: 1, + }, + timestamp: activityTimestamp, + } + _emit(snapshotEvent) + expect(sessionRecording['isIdle']).toEqual(false) + expect(sessionRecording['_lastActivityTimestamp']).toEqual(activityTimestamp) + return snapshotEvent + } + beforeEach(() => { sessionRecording.startRecordingIfEnabled() sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) @@ -1036,26 +1065,25 @@ describe('SessionRecording', () => { size: 0, windowId: null, }) + + // options will have been emitted + expect(_addCustomEvent).toHaveBeenCalled() + _addCustomEvent.mockClear() }) it("enters idle state within one session if the activity is non-user generated and there's no activity for (RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) 5 minutes", () => { const firstActivityTimestamp = startingTimestamp + 100 - const firstSnapshotEvent = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 1, - }, - timestamp: firstActivityTimestamp, - } - _emit(firstSnapshotEvent) - expect(sessionRecording['isIdle']).toEqual(false) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) + const secondActivityTimestamp = startingTimestamp + 200 + const thirdActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000 + const fourthActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 + const firstSnapshotEvent = emitActiveEvent(firstActivityTimestamp) + // event was active so activity timestamp is updated + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - const firstSessionId = sessionRecording['sessionId'] // after the first emit the buffer has been initialised but not flushed + const firstSessionId = sessionRecording['sessionId'] expect(sessionRecording['buffer']).toEqual({ data: [createFullSnapshot(), firstSnapshotEvent], sessionId: firstSessionId, @@ -1063,17 +1091,13 @@ describe('SessionRecording', () => { windowId: expect.any(String), }) - const secondActivityTimestamp = startingTimestamp + 200 - const secondSnapshot = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 0, - }, - timestamp: secondActivityTimestamp, - } - _emit(secondSnapshot) - expect(sessionRecording['isIdle']).toEqual(false) + // the session id generator returns a fixed value, but we want it to rotate in part of this test + sessionIdGeneratorMock.mockClear() + const rotatedSessionId = 'rotated-session-id' + sessionIdGeneratorMock.mockImplementation(() => rotatedSessionId) + + const secondSnapshot = emitInactiveEvent(secondActivityTimestamp, false) + // event was not active so activity timestamp is not updated expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) @@ -1086,46 +1110,32 @@ describe('SessionRecording', () => { }) // this triggers idle state and isn't a user interaction so does not take a full snapshot - const thirdActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 1000 - const thirdSnapshot = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 0, - }, - timestamp: thirdActivityTimestamp, - } - _emit(thirdSnapshot) - expect(sessionRecording['isIdle']).toEqual(true) + emitInactiveEvent(thirdActivityTimestamp, true) expect(_addCustomEvent).toHaveBeenCalledWith('sessionIdle', { reason: 'user inactivity', threshold: 300000, timeSinceLastActive: 300900, }) + // event was not active so activity timestamp is not updated expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - // the third snapshot is dropped since it switches the session to idle // the custom event doesn't show here since there's not a real rrweb to emit it expect(sessionRecording['buffer']).toEqual({ - data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], + data: [ + createFullSnapshot(), + firstSnapshotEvent, + secondSnapshot, + // the third snapshot is dropped since it switches the session to idle + ], sessionId: firstSessionId, size: 156, windowId: expect.any(String), }) // this triggers exit from idle state _and_ is a user interaction, so we take a full snapshot - const fourthActivityTimestamp = startingTimestamp + RECORDING_IDLE_ACTIVITY_TIMEOUT_MS + 2000 - const fourthSnapshot = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 1, - }, - timestamp: fourthActivityTimestamp, - } - _emit(fourthSnapshot) - expect(sessionRecording['isIdle']).toEqual(false) + + const fourthSnapshot = emitActiveEvent(fourthActivityTimestamp) expect(_addCustomEvent).toHaveBeenCalledWith('sessionNoLongerIdle', { reason: 'user activity', type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, @@ -1133,7 +1143,7 @@ describe('SessionRecording', () => { expect(sessionRecording['_lastActivityTimestamp']).toEqual(fourthActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) - // the fourth snapshot should trigger a flush because the session id has changed... + // the fourth snapshot should not trigger a flush because the session id has not changed... expect(sessionRecording['buffer']).toEqual({ // as we return from idle we will capture a full snapshot _before_ the fourth snapshot data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot, createFullSnapshot(), fourthSnapshot], @@ -1148,31 +1158,18 @@ describe('SessionRecording', () => { }) it('rotates session if idle for (MAX_SESSION_IDLE_TIMEOUT) 30 minutes', () => { - // the buffer starts out empty - expect(sessionRecording['buffer']).toEqual({ - data: [], - sessionId: null, - size: 0, - windowId: null, - }) - const firstActivityTimestamp = startingTimestamp + 100 - const firstSnapshotEvent = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 1, - }, - timestamp: firstActivityTimestamp, - } - _emit(firstSnapshotEvent) - expect(sessionRecording['isIdle']).toEqual(false) - expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) + const secondActivityTimestamp = startingTimestamp + 200 + const thirdActivityTimestamp = sessionManager['_sessionTimeoutMs'] + startingTimestamp + 1 + const fourthActivityTimestamp = sessionManager['_sessionTimeoutMs'] + startingTimestamp + 1000 + const firstSnapshotEvent = emitActiveEvent(firstActivityTimestamp) + // event was active so activity timestamp is updated + expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - const firstSessionId = sessionRecording['sessionId'] // after the first emit the buffer has been initialised but not flushed + const firstSessionId = sessionRecording['sessionId'] expect(sessionRecording['buffer']).toEqual({ data: [createFullSnapshot(), firstSnapshotEvent], sessionId: firstSessionId, @@ -1180,26 +1177,15 @@ describe('SessionRecording', () => { windowId: expect.any(String), }) - // the session id generator returns a fixed value but we want it to rotate in part of this test + // the session id generator returns a fixed value, but we want it to rotate in part of this test sessionIdGeneratorMock.mockClear() const rotatedSessionId = 'rotated-session-id' sessionIdGeneratorMock.mockImplementation(() => rotatedSessionId) - const secondActivityTimestamp = startingTimestamp + 200 - const secondSnapshot = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 0, - }, - timestamp: secondActivityTimestamp, - } - _emit(secondSnapshot) - expect(sessionRecording['isIdle']).toEqual(false) - // TODO why is this not the second activity timestamp 🙃 + const secondSnapshot = emitInactiveEvent(secondActivityTimestamp, false) + // event was not active so activity timestamp is not updated expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - expect(sessionIdGeneratorMock).not.toHaveBeenCalled() // the second snapshot remains buffered in memory expect(sessionRecording['buffer']).toEqual({ @@ -1210,37 +1196,26 @@ describe('SessionRecording', () => { }) // this triggers idle state and isn't a user interaction so does not take a full snapshot - const thirdActivityTimestamp = sessionManager['_sessionTimeoutMs'] + startingTimestamp + 1 - const thirdSnapshot = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 0, - }, - // MAX_SESSION_IDLE_TIMEOUT is 30 minutes in _SECONDS_ - timestamp: thirdActivityTimestamp, - } - _addCustomEvent.mockClear() - _emit(thirdSnapshot) - expect(sessionRecording['isIdle']).toEqual(true) - expect(_addCustomEvent.mock.calls).toEqual([ - [ - 'sessionIdle', - { - reason: 'user inactivity', - threshold: 300000, - timeSinceLastActive: 1799901, - }, - ], - ]) + + emitInactiveEvent(thirdActivityTimestamp, true) + expect(_addCustomEvent).toHaveBeenCalledWith('sessionIdle', { + reason: 'user inactivity', + threshold: 300000, + timeSinceLastActive: 1799901, + }) + // event was not active so activity timestamp is not updated expect(sessionRecording['_lastActivityTimestamp']).toEqual(firstActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(1) - expect(sessionIdGeneratorMock).not.toHaveBeenCalled() // the third snapshot is dropped since it switches the session to idle // the custom event doesn't show here since there's not a real rrweb to emit it expect(sessionRecording['buffer']).toEqual({ - data: [createFullSnapshot(), firstSnapshotEvent, secondSnapshot], + data: [ + createFullSnapshot(), + firstSnapshotEvent, + secondSnapshot, + // the third snapshot is dropped since it switches the session to idle + ], sessionId: firstSessionId, size: 156, windowId: expect.any(String), @@ -1250,26 +1225,17 @@ describe('SessionRecording', () => { expect(posthog.capture).not.toHaveBeenCalled() // this triggers exit from idle state _and_ is a user interaction, so we take a full snapshot - const fourthActivityTimestamp = sessionManager['_sessionTimeoutMs'] + 1000 - const fourthSnapshot = { - event: 123, - type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, - data: { - source: 1, - }, - timestamp: fourthActivityTimestamp, - } - _emit(fourthSnapshot) - expect(sessionRecording['isIdle']).toEqual(false) + + const fourthSnapshot = emitActiveEvent(fourthActivityTimestamp) expect(_addCustomEvent).toHaveBeenCalledWith('sessionNoLongerIdle', { reason: 'user activity', type: INCREMENTAL_SNAPSHOT_EVENT_TYPE, }) expect(sessionRecording['_lastActivityTimestamp']).toEqual(fourthActivityTimestamp) expect(assignableWindow.rrwebRecord.takeFullSnapshot).toHaveBeenCalledTimes(2) - expect(sessionIdGeneratorMock).toHaveBeenCalled() // the fourth snapshot causes the session id to change + expect(sessionIdGeneratorMock).toHaveBeenCalledTimes(1) const endingSessionId = sessionRecording['sessionId'] expect(endingSessionId).toEqual(rotatedSessionId) @@ -1294,7 +1260,7 @@ describe('SessionRecording', () => { expect(sessionRecording['buffer']).toEqual({ data: [createFullSnapshot(), fourthSnapshot], sessionId: rotatedSessionId, - size: 82, + size: 88, windowId: expect.any(String), }) }) From 8a8bddd70dde488e0d514d391cfbcc0c2f5f6a2b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 11 Jan 2024 14:29:45 +0000 Subject: [PATCH 7/7] fix --- .../extensions/replay/sessionrecording.test.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index e644485fe..bb86771f7 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -251,20 +251,10 @@ describe('SessionRecording', () => { expect(sessionRecording['status']).toBe('buffering') expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) - _emit(createIncrementalSnapshot({ data: { source: 1 } })) + const incrementalSnapshot = createIncrementalSnapshot({ data: { source: 1 } }) + _emit(incrementalSnapshot) expect(sessionRecording['buffer']).toEqual({ - data: [ - { - data: {}, - type: 2, - }, - { - data: { - source: 1, - }, - type: 3, - }, - ], + data: [createFullSnapshot(), incrementalSnapshot], sessionId: sessionId, size: 50, windowId: 'windowId',