fix: recordings that are paused for their whole duration (#1626)

PostHog · Dec 20, 2024 · 9eaf9bc · 9eaf9bc
1 parent 91290c7
commit 9eaf9bc
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 33 deletions.
diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts
@@ -47,7 +47,6 @@ import {
 } from '@rrweb/types'
 import Mock = jest.Mock
 import { ConsentManager } from '../../../consent'
-import { waitFor } from '@testing-library/preact'
 import { SimpleEventEmitter } from '../../../utils/simple-event-emitter'
 
 // Type and source defined here designate a non-user-generated recording event
@@ -2317,48 +2316,55 @@ describe('SessionRecording', () => {
             )
         })
 
-        it('flushes buffer and includes pause event when hitting blocked URL', async () => {
+        it('does not flush buffer and includes pause event when hitting blocked URL', async () => {
             // Emit some events before hitting blocked URL
             _emit(createIncrementalSnapshot({ data: { source: 1 } }))
             _emit(createIncrementalSnapshot({ data: { source: 2 } }))
 
             // Simulate URL change to blocked URL
             fakeNavigateTo('https://test.com/blocked')
-            _emit(createIncrementalSnapshot({ data: { source: 3 } }))
-            expect(document.body).toHaveClass('ph-no-capture')
 
-            await waitFor(() => {
-                // Verify the buffer was flushed with all events including pause
-                expect(posthog.capture).toHaveBeenCalledWith(
-                    '$snapshot',
-                    {
-                        $session_id: sessionId,
-                        $window_id: 'windowId',
-                        $snapshot_bytes: expect.any(Number),
-                        $snapshot_data: [
-                            { type: 3, data: { source: 1 } },
-                            { type: 3, data: { source: 2 } },
-                        ],
-                        $lib: 'web',
-                        $lib_version: '0.0.1',
-                    },
-                    expect.any(Object)
-                )
-            })
+            expect(posthog.capture).not.toHaveBeenCalled()
 
             // Verify subsequent events are not captured while on blocked URL
+            _emit(createIncrementalSnapshot({ data: { source: 3 } }))
             _emit(createIncrementalSnapshot({ data: { source: 4 } }))
-            expect(sessionRecording['buffer'].data).toHaveLength(0)
+
+            expect(sessionRecording['buffer'].data).toEqual([
+                {
+                    data: {
+                        source: 1,
+                    },
+                    type: 3,
+                },
+                {
+                    data: {
+                        source: 2,
+                    },
+                    type: 3,
+                },
+            ])
 
             // Simulate URL change to allowed URL
             fakeNavigateTo('https://test.com/allowed')
 
             // Verify recording resumes with resume event
             _emit(createIncrementalSnapshot({ data: { source: 5 } }))
 
-            expect(document.body).not.toHaveClass('ph-no-capture')
-
             expect(sessionRecording['buffer'].data).toStrictEqual([
+                {
+                    data: {
+                        source: 1,
+                    },
+                    type: 3,
+                },
+                {
+                    data: {
+                        source: 2,
+                    },
+                    type: 3,
+                },
+                // restarts with a snapshot
                 expect.objectContaining({
                     type: 2,
                 }),

diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts
@@ -1175,7 +1175,7 @@ export class SessionRecording {
         const isBelowMinimumDuration =
             isNumber(minimumDuration) && isPositiveSessionDuration && sessionDuration < minimumDuration
 
-        if (this.status === 'buffering' || isBelowMinimumDuration) {
+        if (this.status === 'buffering' || this.status === 'paused' || isBelowMinimumDuration) {
             this.flushBufferTimer = setTimeout(() => {
                 this._flushBuffer()
             }, RECORDING_BUFFER_TIMEOUT)
@@ -1270,17 +1270,15 @@ export class SessionRecording {
             return
         }
 
+        // we can't flush the buffer here since someone might be starting on a blocked page,
+        // and we need to be sure that we don't record that page
+        // so we might not get the below custom event but events will report the paused status
+        // which will allow debugging of sessions that start on blocked pages
         this._urlBlocked = true
-        document?.body?.classList?.add('ph-no-capture')
 
         // Clear the snapshot timer since we don't want new snapshots while paused
         clearInterval(this._fullSnapshotTimer)
 
-        // Running this in a timeout to ensure we can
-        setTimeout(() => {
-            this._flushBuffer()
-        }, 100)
-
         logger.info('recording paused due to URL blocker')
         this._tryAddCustomEvent('recording paused', { reason: 'url blocker' })
     }
@@ -1291,7 +1289,6 @@ export class SessionRecording {
         }
 
         this._urlBlocked = false
-        document?.body?.classList?.remove('ph-no-capture')
 
         this._tryTakeFullSnapshot()
         this._scheduleFullSnapshot()