From bdd18771b64b66faeecd583c7208e3f9d505bdf7 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 14 Nov 2023 12:13:53 +0000 Subject: [PATCH 1/2] fix: a little session buffering logic --- .../replay/sessionrecording.test.ts | 24 +++++++++++++++++++ src/extensions/replay/sessionrecording.ts | 5 +++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 42cb8b05a..1612c8711 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -1108,6 +1108,30 @@ describe('SessionRecording', () => { expect(posthog.capture).not.toHaveBeenCalled() }) + it('does flush if session duration is negative', () => { + sessionRecording.afterDecideResponse( + makeDecideResponse({ + sessionRecording: { minimumDurationMilliseconds: 1500 }, + }) + ) + sessionRecording.startRecordingIfEnabled() + expect(sessionRecording['status']).toBe('active') + const { sessionStartTimestamp } = sessionManager.checkAndGetSessionAndWindowId(true) + + // if we have some data in the buffer and the buffer has a session id but then the session id changes + // then the session duration will be negative and we will never flush the buffer + _emit(createIncrementalSnapshot({ data: { source: 1 }, timestamp: sessionStartTimestamp - 1000 })) + + expect(sessionRecording['sessionDuration']).toBe(-1000) + expect(sessionRecording['_minimumDuration']).toBe(1500) + + expect(sessionRecording['buffer']?.data.length).toBe(1) + // call the private method to avoid waiting for the timer + sessionRecording['_flushBuffer']() + + expect(posthog.capture).toHaveBeenCalled() + }) + it('does not stay buffering after the minimum duration', () => { sessionRecording.afterDecideResponse( makeDecideResponse({ diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index d846c5ae2..7814f4454 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -602,8 +602,11 @@ export class SessionRecording { const minimumDuration = this._minimumDuration const sessionDuration = this.sessionDuration + // if we have old data in the buffer but the session has rotated then the + // session duration might be negative, in that case we want to flush the buffer + const isPositiveSessionDuration = _isNumber(sessionDuration) && sessionDuration >= 0 const isBelowMinimumDuration = - _isNumber(minimumDuration) && _isNumber(sessionDuration) && sessionDuration < minimumDuration + _isNumber(minimumDuration) && isPositiveSessionDuration && sessionDuration < minimumDuration if (this.status === 'buffering' || isBelowMinimumDuration) { this.flushBufferTimer = setTimeout(() => { From f16bbad40e733713f3ac4a54e252230dd64aa7d7 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 14 Nov 2023 12:16:40 +0000 Subject: [PATCH 2/2] Fix --- src/__tests__/extensions/replay/sessionrecording.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 1612c8711..c82747c24 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -1119,7 +1119,8 @@ describe('SessionRecording', () => { const { sessionStartTimestamp } = sessionManager.checkAndGetSessionAndWindowId(true) // if we have some data in the buffer and the buffer has a session id but then the session id changes - // then the session duration will be negative and we will never flush the buffer + // then the session duration will be negative, and we will never flush the buffer + // this setup isn't quite that but does simulate the behaviour closely enough _emit(createIncrementalSnapshot({ data: { source: 1 }, timestamp: sessionStartTimestamp - 1000 })) expect(sessionRecording['sessionDuration']).toBe(-1000)