diff --git a/cypress/e2e/session-recording.cy.ts b/cypress/e2e/session-recording.cy.ts index 25797af31..312dacb4a 100644 --- a/cypress/e2e/session-recording.cy.ts +++ b/cypress/e2e/session-recording.cy.ts @@ -397,10 +397,18 @@ describe('Session recording', () => { // a meta and then a full snapshot expect(captures[1]['properties']['$snapshot_data'][0].type).to.equal(4) // meta expect(captures[1]['properties']['$snapshot_data'][1].type).to.equal(2) // full_snapshot - expect(captures[1]['properties']['$snapshot_data'][2].type).to.equal(5) // custom event with options - expect(captures[1]['properties']['$snapshot_data'][3].type).to.equal(5) // custom event with posthog config + + expect(captures[1]['properties']['$snapshot_data'][2].type).to.equal(5) + expect(captures[1]['properties']['$snapshot_data'][2].data.tag).to.equal('$pageview') + + expect(captures[1]['properties']['$snapshot_data'][3].type).to.equal(5) // custom event with options + expect(captures[1]['properties']['$snapshot_data'][3].data.tag).to.equal('$session_options') + + expect(captures[1]['properties']['$snapshot_data'][4].type).to.equal(5) // custom event with posthog config + expect(captures[1]['properties']['$snapshot_data'][4].data.tag).to.equal('$posthog_config') // custom event with posthog config + const xPositions = [] - for (let i = 4; i < captures[1]['properties']['$snapshot_data'].length; i++) { + for (let i = 5; i < captures[1]['properties']['$snapshot_data'].length; i++) { expect(captures[1]['properties']['$snapshot_data'][i].type).to.equal(3) expect(captures[1]['properties']['$snapshot_data'][i].data.source).to.equal( 6, diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index 4faac3be9..7cc74b3b8 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -129,6 +129,8 @@ describe('SessionRecording', () => { let sessionIdGeneratorMock: Mock let windowIdGeneratorMock: Mock let onFeatureFlagsCallback: ((flags: string[], variants: Record) => void) | null + let removeCaptureHookMock: Mock + let addCaptureHookMock: Mock const addRRwebToWindow = () => { assignableWindow.rrweb = { @@ -173,6 +175,10 @@ describe('SessionRecording', () => { sessionManager = new SessionIdManager(config, postHogPersistence, sessionIdGeneratorMock, windowIdGeneratorMock) + // add capture hook returns an unsubscribe function + removeCaptureHookMock = jest.fn() + addCaptureHookMock = jest.fn().mockImplementation(() => removeCaptureHookMock) + posthog = { get_property: (property_key: string): Property | undefined => { return postHogPersistence?.['props'][property_key] @@ -185,7 +191,7 @@ describe('SessionRecording', () => { }, sessionManager: sessionManager, requestRouter: new RequestRouter({ config } as any), - _addCaptureHook: jest.fn(), + _addCaptureHook: addCaptureHookMock, consent: { isOptedOut: () => false }, } as unknown as PostHog @@ -308,6 +314,44 @@ describe('SessionRecording', () => { expect((sessionRecording as any)._startCapture).toHaveBeenCalled() }) + it('sets the pageview capture hook once', () => { + expect(sessionRecording['_removePageViewCaptureHook']).toBeUndefined() + + sessionRecording.startIfEnabledOrStop() + + expect(sessionRecording['_removePageViewCaptureHook']).not.toBeUndefined() + expect(posthog._addCaptureHook).toHaveBeenCalledTimes(1) + + // calling a second time doesn't add another capture hook + sessionRecording.startIfEnabledOrStop() + expect(posthog._addCaptureHook).toHaveBeenCalledTimes(1) + }) + + it('removes the pageview capture hook on stop', () => { + sessionRecording.startIfEnabledOrStop() + expect(sessionRecording['_removePageViewCaptureHook']).not.toBeUndefined() + + expect(removeCaptureHookMock).not.toHaveBeenCalled() + sessionRecording.stopRecording() + + expect(removeCaptureHookMock).toHaveBeenCalledTimes(1) + expect(sessionRecording['_removePageViewCaptureHook']).toBeUndefined() + }) + + it('sets the window event listeners', () => { + //mock window add event listener to check if it is called + const addEventListener = jest.fn().mockImplementation(() => () => {}) + window.addEventListener = addEventListener + + sessionRecording.startIfEnabledOrStop() + expect(sessionRecording['_onBeforeUnload']).not.toBeNull() + // we register 4 event listeners + expect(window.addEventListener).toHaveBeenCalledTimes(4) + + // window.addEventListener('blah', someFixedListenerInstance) is safe to call multiple times, + // so we don't need to test if the addEvenListener registrations are called multiple times + }) + it('emits an options event', () => { sessionRecording.startIfEnabledOrStop() expect((sessionRecording as any)['_tryAddCustomEvent']).toHaveBeenCalledWith('$session_options', { diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 14caeb2fe..58646fb26 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -116,6 +116,7 @@ export class SessionRecording { private _fullSnapshotTimer?: ReturnType + private _removePageViewCaptureHook: (() => void) | undefined = undefined // if pageview capture is disabled // then we can manually track href changes private _lastHref?: string @@ -284,16 +285,35 @@ export class SessionRecording { if (this.isRecordingEnabled) { this._startCapture() + // calling addEventListener multiple times is safe and will not add duplicates window?.addEventListener('beforeunload', this._onBeforeUnload) window?.addEventListener('offline', this._onOffline) window?.addEventListener('online', this._onOnline) window?.addEventListener('visibilitychange', this._onVisibilityChange) + 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) => { + // 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 (!href) { + return + } + this._tryAddCustomEvent('$pageview', { href }) + } + } catch (e) { + logger.error('Could not add $pageview to rrweb session', e) + } + }) + } + logger.info(LOGGER_PREFIX + ' started') } else { this.stopRecording() - this.clearBuffer() - clearInterval(this._fullSnapshotTimer) } } @@ -308,6 +328,12 @@ export class SessionRecording { window?.removeEventListener('online', this._onOnline) window?.removeEventListener('visibilitychange', this._onVisibilityChange) + this.clearBuffer() + clearInterval(this._fullSnapshotTimer) + + this._removePageViewCaptureHook?.() + this._removePageViewCaptureHook = undefined + logger.info(LOGGER_PREFIX + ' stopped') } } @@ -661,24 +687,6 @@ export class SessionRecording { ...sessionRecordingOptions, }) - // :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.instance._addCaptureHook((eventName) => { - // 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 (!href) { - return - } - this._tryAddCustomEvent('$pageview', { href }) - } - } catch (e) { - logger.error('Could not add $pageview to rrweb session', e) - } - }) - // We reset the last activity timestamp, resetting the idle timer this._lastActivityTimestamp = Date.now() this.isIdle = false diff --git a/src/posthog-core.ts b/src/posthog-core.ts index d8cf8c3d9..adce9188c 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -864,8 +864,8 @@ export class PostHog { return data } - _addCaptureHook(callback: (eventName: string, eventPayload?: CaptureResult) => void): void { - this.on('eventCaptured', (data) => callback(data.event, data)) + _addCaptureHook(callback: (eventName: string, eventPayload?: CaptureResult) => void): () => void { + return this.on('eventCaptured', (data) => callback(data.event, data)) } _calculate_event_properties(event_name: string, event_properties: Properties, timestamp?: Date): Properties {