From c2d3631f67f8113f875a4048a50b55cb312482cc Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sat, 1 Jun 2024 08:50:43 +0100 Subject: [PATCH 1/2] fix: do not start recording buffer without id --- .../replay/sessionrecording.test.ts | 61 ++++++++++++++----- src/extensions/replay/sessionrecording.ts | 12 ++-- 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index b92f06f97..4a2058993 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -274,7 +274,11 @@ describe('SessionRecording', () => { sessionRecording.startIfEnabledOrStop() expect(loadScript).toHaveBeenCalled() expect(sessionRecording['status']).toBe('buffering') - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual({ + ...EMPTY_BUFFER, + sessionId: sessionId, + windowId: 'windowId', + }) const metaSnapshot = createMetaSnapshot({ data: { href: 'https://example.com' } }) _emit(metaSnapshot) @@ -290,7 +294,11 @@ describe('SessionRecording', () => { sessionRecording.startIfEnabledOrStop() expect(loadScript).toHaveBeenCalled() expect(sessionRecording['status']).toBe('buffering') - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual({ + ...EMPTY_BUFFER, + sessionId: sessionId, + windowId: 'windowId', + }) const fullSnapshot = createFullSnapshot() _emit(fullSnapshot) @@ -306,7 +314,11 @@ describe('SessionRecording', () => { sessionRecording.startIfEnabledOrStop() expect(loadScript).toHaveBeenCalled() expect(sessionRecording['status']).toBe('buffering') - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual({ + ...EMPTY_BUFFER, + sessionId: sessionId, + windowId: 'windowId', + }) const incrementalSnapshot = createIncrementalSnapshot({ data: { source: 1 } }) _emit(incrementalSnapshot) @@ -322,7 +334,11 @@ describe('SessionRecording', () => { sessionRecording.startIfEnabledOrStop() expect(loadScript).toHaveBeenCalled() expect(sessionRecording['status']).toBe('buffering') - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual({ + ...EMPTY_BUFFER, + sessionId: sessionId, + windowId: 'windowId', + }) const incrementalSnapshot = createIncrementalSnapshot({ data: { source: 1 } }) _emit(incrementalSnapshot) @@ -717,7 +733,7 @@ describe('SessionRecording', () => { sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() - expect(sessionRecording['buffer']?.sessionId).toEqual(null) + expect(sessionRecording['buffer']?.sessionId).toEqual(sessionId) _emit(createIncrementalSnapshot({ emit: 1 })) @@ -883,6 +899,11 @@ describe('SessionRecording', () => { it('does not send a full snapshot if there is not a new session or window id', () => { assignableWindow.rrweb.record.takeFullSnapshot.mockClear() + // we always take a full snapshot when there hasn't been one + // and use _fullSnapshotTimer to track that + // we want to avoid that behavior here, so we set it to any value + sessionRecording['_fullSnapshotTimer'] = 1 + sessionIdGeneratorMock.mockImplementation(() => 'old-session-id') windowIdGeneratorMock.mockImplementation(() => 'old-window-id') sessionManager.resetSessionId() @@ -1153,9 +1174,9 @@ describe('SessionRecording', () => { // the buffer starts out empty expect(sessionRecording['buffer']).toEqual({ data: [], - sessionId: null, + sessionId: sessionId, size: 0, - windowId: null, + windowId: 'windowId', }) // options will have been emitted @@ -1164,22 +1185,32 @@ describe('SessionRecording', () => { }) it('does not emit when idle', () => { + const emptyBuffer = { + ...EMPTY_BUFFER, + sessionId: sessionId, + windowId: 'windowId', + } + // force idle state sessionRecording['isIdle'] = true // buffer is empty - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual(emptyBuffer) // a plugin event doesn't count as returning from idle sessionRecording.onRRwebEmit(createPluginSnapshot({}) as unknown as eventWithTime) // buffer is still empty - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual(emptyBuffer) }) it('emits custom events even when idle', () => { // force idle state sessionRecording['isIdle'] = true // buffer is empty - expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER) + expect(sessionRecording['buffer']).toEqual({ + ...EMPTY_BUFFER, + sessionId: sessionId, + windowId: 'windowId', + }) sessionRecording.onRRwebEmit(createCustomSnapshot({}) as unknown as eventWithTime) @@ -1194,9 +1225,9 @@ describe('SessionRecording', () => { type: 5, }, ], - sessionId: null, + sessionId: sessionId, size: 47, - windowId: null, + windowId: 'windowId', }) }) @@ -1609,12 +1640,12 @@ describe('SessionRecording', () => { expect(sessionRecording['_fullSnapshotTimer']).toBe(undefined) }) - it('schedules a snapshot on start', () => { + it('does not schedule a snapshot on start', () => { sessionRecording.startIfEnabledOrStop() - expect(sessionRecording['_fullSnapshotTimer']).not.toBe(undefined) + expect(sessionRecording['_fullSnapshotTimer']).toBe(undefined) }) - it('reschedules a snapshot, when we take a full snapshot', () => { + it('schedules a snapshot, when we take a full snapshot', () => { sessionRecording.startIfEnabledOrStop() const startTimer = sessionRecording['_fullSnapshotTimer'] diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 2f696a176..a2caa7027 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -281,6 +281,11 @@ export class SessionRecording { throw new Error(LOGGER_PREFIX + ' started without valid sessionManager. This is a bug.') } + // we know there's a sessionManager, so don't need to start without a session id + const { sessionId, windowId } = this.sessionManager.checkAndGetSessionAndWindowId() + this.sessionId = sessionId + this.windowId = windowId + this.buffer = this.clearBuffer() // on reload there might be an already sampled session that should be continued before decide response, @@ -554,7 +559,7 @@ export class SessionRecording { if ( returningFromIdle || ([FULL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE].indexOf(event.type) === -1 && - (windowIdChanged || sessionIdChanged)) + (windowIdChanged || sessionIdChanged || isUndefined(this._fullSnapshotTimer))) ) { this._tryTakeFullSnapshot() } @@ -645,11 +650,6 @@ export class SessionRecording { }, }) - // rrweb takes a snapshot on initialization, - // we want to take one in five minutes - // if nothing else happens to reset the timer - this._scheduleFullSnapshot() - const activePlugins = this._gatherRRWebPlugins() this.stopRrweb = this.rrwebRecord({ emit: (event) => { From 022253b45cce72fa996daabb45f577dd277573bc Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Sat, 1 Jun 2024 17:48:55 +0100 Subject: [PATCH 2/2] correct field declaration --- src/extensions/replay/sessionrecording.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index a2caa7027..b467d6ef9 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -129,8 +129,8 @@ export class SessionRecording { private _linkedFlagSeen: boolean = false private _lastActivityTimestamp: number = Date.now() - private windowId: string | null = null - private sessionId: string | null = null + private windowId: string + private sessionId: string private _linkedFlag: string | FlagVariant | null = null private _fullSnapshotTimer?: ReturnType