properly handle whether session recording was already in progress

Author: luke-belton

packages/browser/src/__tests__/extensions/feedback-recording.test.ts

@@ -32,14 +32,16 @@ describe('FeedbackRecordingManager', () => {
             config: {
                 api_host: 'https://test.com',
                 token: 'test-token',
-            },
+            } as any,
             capture: jest.fn(),
             startSessionRecording: jest.fn(),
+            stopSessionRecording: jest.fn(),
+            sessionRecordingStarted: jest.fn(),
             get_session_id: jest.fn().mockReturnValue('mock-session-id'),
             _send_request: jest.fn(),
             requestRouter: {
                 endpointFor: jest.fn().mockReturnValue('https://test.com/api/feedback/audio'),
-            },
+            } as any,
         })
 
         audioRecorderMock = {
@@ -166,6 +168,8 @@ describe('FeedbackRecordingManager', () => {
             let onRecordingEnded: jest.Mock
 
             beforeEach(async () => {
+                jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(false)
+
                 onRecordingEnded = jest.fn()
                 await manager.launchFeedbackRecordingUI(onRecordingEnded)
 
@@ -214,6 +218,22 @@ describe('FeedbackRecordingManager', () => {
                         $feedback_recording_id: feedbackId,
                     })
                 })
+
+                it('starts session recording if not already active', async () => {
+                    jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(false)
+
+                    feedbackId = await handleStartRecording()
+
+                    expect(instance.startSessionRecording).toHaveBeenCalledWith(true)
+                })
+
+                it('does not start session recording if already active', async () => {
+                    jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(true)
+
+                    feedbackId = await handleStartRecording()
+
+                    expect(instance.startSessionRecording).not.toHaveBeenCalled()
+                })
             })
 
             describe('stop', () => {
@@ -334,6 +354,24 @@ describe('FeedbackRecordingManager', () => {
                     expect(instance._send_request).not.toHaveBeenCalled()
                     expect(onRecordingEnded).toHaveBeenCalledTimes(1)
                 })
+
+                it('stops session recording when stopping if we started it', async () => {
+                    jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(false)
+
+                    feedbackId = await handleStartRecording()
+                    await stopCallback(feedbackId)
+
+                    expect(instance.stopSessionRecording).toHaveBeenCalled()
+                })
+
+                it('does not stop session recording if we did not start it', async () => {
+                    jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(true)
+
+                    feedbackId = await handleStartRecording()
+                    await stopCallback(feedbackId)
+
+                    expect(instance.stopSessionRecording).not.toHaveBeenCalled()
+                })
             })
 
             describe('cleanup', () => {
@@ -357,6 +395,26 @@ describe('FeedbackRecordingManager', () => {
                     expect(manager.isFeedbackRecordingActive()).toBe(false)
                     expect(manager.getCurrentFeedbackRecordingId()).toBeNull()
                 })
+
+                it('stops session recording when stopping if we started it', async () => {
+                    jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(false)
+
+                    feedbackId = await handleStartRecording()
+
+                    manager.cleanup()
+
+                    expect(instance.stopSessionRecording).toHaveBeenCalled()
+                })
+
+                it('does not stop session recording if we did not start it', async () => {
+                    jest.spyOn(instance, 'sessionRecordingStarted').mockReturnValue(true)
+
+                    feedbackId = await handleStartRecording()
+
+                    manager.cleanup()
+
+                    expect(instance.stopSessionRecording).not.toHaveBeenCalled()
+                })
             })
 
             describe('UI focus after completion', () => {

packages/browser/src/extensions/feedback-recording.ts

@@ -17,6 +17,7 @@ export class FeedbackRecordingManager {
     private _isLoaded: boolean = false
     private _isLoading: boolean = false
     private _isUIActive: boolean = false
+    private _didStartSessionRecording: boolean = false
 
     constructor(
         private _instance: PostHog,
@@ -39,6 +40,11 @@ export class FeedbackRecordingManager {
             this._audioRecorder.cancelRecording()
         }
 
+        if (this._didStartSessionRecording) {
+            this._instance.stopSessionRecording()
+            logger.info('Stopped session recording during cleanup')
+        }
+
         this._resetState()
         removeFeedbackRecordingUIFromDOM()
 
@@ -127,9 +133,16 @@ export class FeedbackRecordingManager {
             logger.warn('Failed to start audio recording:', error)
         }
 
-        //TODO: at the moment always just start recording - we can mess with this later
-        // by storing whether reocrding is already in progress so we know whether to stop it later
-        this._instance.startSessionRecording(true)
+        // Start session recording if it's not already active
+        const wasSessionRecordingActive = this._instance.sessionRecordingStarted()
+        if (!wasSessionRecordingActive) {
+            this._instance.startSessionRecording(true)
+            this._didStartSessionRecording = true
+            logger.info('Started session recording for feedback')
+        } else {
+            this._didStartSessionRecording = false
+            logger.info('Session recording already active, reusing existing recording')
+        }
 
         return feedbackId
     }
@@ -187,6 +200,11 @@ export class FeedbackRecordingManager {
         feedbackId: string,
         onRecordingEnded: (result: UserFeedbackRecordingResult) => void
     ): Promise<void> {
+        if (this._didStartSessionRecording) {
+            this._instance.stopSessionRecording()
+            logger.info('Stopped session recording after feedback recording ended')
+        }
+
         const recordingResult = await this._audioRecorder.stopRecording()
 
         if (recordingResult?.blob) {
@@ -221,6 +239,7 @@ export class FeedbackRecordingManager {
     private _resetState(): void {
         this._feedbackRecordingId = null
         this._isUIActive = false
+        this._didStartSessionRecording = false
     }
 }