fix: infinite loop when combining controls (#1569)

cypress/e2e/session-recording.cy.ts

@@ -523,6 +523,7 @@ describe('Session recording', () => {
                         capturedSnapshot['properties']['$snapshot_data'][3],
                         capturedSnapshot['properties']['$snapshot_data'][4],
                     ])
+
                     expectPageViewCustomEvent(customEvents[0])
                     expectPostHogConfigCustomEvent(customEvents[1])
                     expectSessionOptionsCustomEvent(customEvents[2])

src/__tests__/extensions/replay/sessionrecording.test.ts

@@ -184,8 +184,7 @@ describe('SessionRecording', () => {
     let sessionIdGeneratorMock: Mock
     let windowIdGeneratorMock: Mock
     let onFeatureFlagsCallback: ((flags: string[], variants: Record<string, string | boolean>) => void) | null
-    let removeCaptureHookMock: Mock
-    let addCaptureHookMock: Mock
+    let removePageviewCaptureHookMock: Mock
     let simpleEventEmitter: SimpleEventEmitter
 
     const addRRwebToWindow = () => {
@@ -208,6 +207,7 @@ describe('SessionRecording', () => {
     }
 
     beforeEach(() => {
+        removePageviewCaptureHookMock = jest.fn()
         sessionId = 'sessionId' + uuidv7()
 
         config = {
@@ -241,10 +241,6 @@ describe('SessionRecording', () => {
             windowIdGeneratorMock
         )
 
-        // add capture hook returns an unsubscribe function
-        removeCaptureHookMock = jest.fn()
-        addCaptureHookMock = jest.fn().mockImplementation(() => removeCaptureHookMock)
-
         simpleEventEmitter = new SimpleEventEmitter()
         // TODO we really need to make this a real posthog instance :cry:
         posthog = {
@@ -262,17 +258,17 @@ describe('SessionRecording', () => {
             },
             sessionManager: sessionManager,
             requestRouter: new RequestRouter({ config } as any),
-            _addCaptureHook: addCaptureHookMock,
             consent: {
                 isOptedOut(): boolean {
                     return false
                 },
             } as unknown as ConsentManager,
             register_for_session() {},
             _internalEventEmitter: simpleEventEmitter,
-            on: (event, cb) => {
-                return simpleEventEmitter.on(event, cb)
-            },
+            on: jest.fn().mockImplementation((event, cb) => {
+                const unsubscribe = simpleEventEmitter.on(event, cb)
+                return removePageviewCaptureHookMock.mockImplementation(unsubscribe)
+            }),
         } as Partial<PostHog> as PostHog
 
         loadScriptMock.mockImplementation((_ph, _path, callback) => {
@@ -423,21 +419,21 @@ describe('SessionRecording', () => {
             sessionRecording.startIfEnabledOrStop()
 
             expect(sessionRecording['_removePageViewCaptureHook']).not.toBeUndefined()
-            expect(posthog._addCaptureHook).toHaveBeenCalledTimes(1)
+            expect(posthog.on).toHaveBeenCalledTimes(1)
 
             // calling a second time doesn't add another capture hook
             sessionRecording.startIfEnabledOrStop()
-            expect(posthog._addCaptureHook).toHaveBeenCalledTimes(1)
+            expect(posthog.on).toHaveBeenCalledTimes(1)
         })
 
         it('removes the pageview capture hook on stop', () => {
             sessionRecording.startIfEnabledOrStop()
             expect(sessionRecording['_removePageViewCaptureHook']).not.toBeUndefined()
 
-            expect(removeCaptureHookMock).not.toHaveBeenCalled()
+            expect(removePageviewCaptureHookMock).not.toHaveBeenCalled()
             sessionRecording.stopRecording()
 
-            expect(removeCaptureHookMock).toHaveBeenCalledTimes(1)
+            expect(removePageviewCaptureHookMock).toHaveBeenCalledTimes(1)
             expect(sessionRecording['_removePageViewCaptureHook']).toBeUndefined()
         })
 
@@ -1774,6 +1770,66 @@ describe('SessionRecording', () => {
             expect(sessionRecording['_linkedFlagSeen']).toEqual(true)
             expect(sessionRecording['status']).toEqual('active')
         })
+
+        /**
+         * this is partly a regression test, with a running rrweb,
+         * if you don't pause while buffering
+         * the browser can be trapped in an infinite loop of pausing
+         * while trying to report it is paused 🙈
+         */
+        it('can be paused while waiting for flag', () => {
+            fakeNavigateTo('https://test.com/blocked')
+
+            expect(sessionRecording['_linkedFlag']).toEqual(null)
+            expect(sessionRecording['_linkedFlagSeen']).toEqual(false)
+            expect(sessionRecording['status']).toEqual('buffering')
+
+            sessionRecording.afterDecideResponse(
+                makeDecideResponse({
+                    sessionRecording: {
+                        endpoint: '/s/',
+                        linkedFlag: 'the-flag-key',
+                        urlBlocklist: [
+                            {
+                                matching: 'regex',
+                                url: '/blocked',
+                            },
+                        ],
+                    },
+                })
+            )
+
+            expect(sessionRecording['_linkedFlag']).toEqual('the-flag-key')
+            expect(sessionRecording['_linkedFlagSeen']).toEqual(false)
+            expect(sessionRecording['status']).toEqual('buffering')
+            expect(sessionRecording['paused']).toBeUndefined()
+
+            const snapshotEvent = {
+                event: 123,
+                type: INCREMENTAL_SNAPSHOT_EVENT_TYPE,
+                data: {
+                    source: 1,
+                },
+                timestamp: new Date().getTime(),
+            }
+            _emit(snapshotEvent)
+
+            expect(sessionRecording['_linkedFlag']).toEqual('the-flag-key')
+            expect(sessionRecording['_linkedFlagSeen']).toEqual(false)
+            expect(sessionRecording['status']).toEqual('paused')
+
+            sessionRecording.overrideLinkedFlag()
+
+            expect(sessionRecording['_linkedFlagSeen']).toEqual(true)
+            expect(sessionRecording['status']).toEqual('paused')
+
+            fakeNavigateTo('https://test.com/allowed')
+
+            expect(sessionRecording['status']).toEqual('paused')
+
+            _emit(snapshotEvent)
+            expect(sessionRecording['status']).toEqual('active')
+        })
     })
 
     describe('buffering minimum duration', () => {

src/extensions/replay/sessionrecording.ts

@@ -392,6 +392,10 @@ export class SessionRecording {
             return 'disabled'
         }
 
+        if (this._urlBlocked) {
+            return 'paused'
+        }
+
         if (!isNullish(this._linkedFlag) && !this._linkedFlagSeen) {
             return 'buffering'
         }
@@ -400,10 +404,6 @@ export class SessionRecording {
             return 'buffering'
         }
 
-        if (this._urlBlocked) {
-            return 'paused'
-        }
-
         if (isBoolean(this.isSampled)) {
             return this.isSampled ? 'sampled' : 'disabled'
         } else {
@@ -501,12 +501,14 @@ export class SessionRecording {
             if (isNullish(this._removePageViewCaptureHook)) {
                 // :TRICKY: rrweb does not capture navigation within SPA-s, so hook into our $pageview events to get access to all events.
                 //   Dropping the initial event is fine (it's always captured by rrweb).
-                this._removePageViewCaptureHook = this.instance._addCaptureHook((eventName) => {
+                this._removePageViewCaptureHook = this.instance.on('eventCaptured', (event) => {
                     // If anything could go wrong here it has the potential to block the main loop,
                     // so we catch all errors.
                     try {
-                        if (eventName === '$pageview') {
-                            const href = window ? this._maskUrl(window.location.href) : ''
+                        if (event.event === '$pageview') {
+                            const href = event?.properties.$current_url
+                                ? this._maskUrl(event?.properties.$current_url)
+                                : ''
                             if (!href) {
                                 return
                             }