Skip to content

Commit 4ab2dc8

Browse files
Kim DuganKim Dugan
authored andcommitted
replay: wait for new config before recording decision
1 parent 755c2d7 commit 4ab2dc8

File tree

3 files changed

+74
-0
lines changed

3 files changed

+74
-0
lines changed

packages/browser/src/__tests__/extensions/replay/sessionRecording-onRemoteConfig.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,5 +374,66 @@ describe('SessionRecording', () => {
374374
expect(posthog.get_property(SESSION_RECORDING_REMOTE_CONFIG).enabled).toBe(true)
375375
expect(sessionRecording['_lazyLoadedSessionRecording']['_endpoint']).toEqual('/ses/')
376376
})
377+
378+
it('does not start recording before remote config is received', () => {
379+
// Set stale config in persistence (simulating cached/CDN config)
380+
posthog.persistence?.register({
381+
[SESSION_RECORDING_REMOTE_CONFIG]: {
382+
enabled: true,
383+
endpoint: '/s/',
384+
urlTriggers: [],
385+
},
386+
})
387+
388+
// Try to start without receiving remote config
389+
sessionRecording.startIfEnabledOrStop()
390+
391+
// Should not have started recording yet
392+
expect(loadScriptMock).not.toHaveBeenCalled()
393+
expect(sessionRecording.status).toBe('lazy_loading')
394+
395+
// Now receive remote config
396+
sessionRecording.onRemoteConfig(
397+
makeFlagsResponse({
398+
sessionRecording: { endpoint: '/s/' },
399+
})
400+
)
401+
402+
// Should now start
403+
expect(loadScriptMock).toHaveBeenCalled()
404+
expect(sessionRecording.status).toBe('active')
405+
})
406+
407+
it('discards buffer on beforeunload if remote config not yet loaded', () => {
408+
// Set persistence to simulate config exists
409+
posthog.persistence?.register({
410+
[SESSION_RECORDING_REMOTE_CONFIG]: {
411+
enabled: true,
412+
endpoint: '/s/',
413+
},
414+
})
415+
416+
// Receive config to start recording
417+
sessionRecording.onRemoteConfig(
418+
makeFlagsResponse({
419+
sessionRecording: { endpoint: '/s/', urlTriggers: [{ url: 'example.com', matching: 'regex' }] },
420+
})
421+
)
422+
423+
// Should be buffering (waiting for trigger)
424+
expect(sessionRecording.status).toBe('buffering')
425+
426+
const lazyRecorder = sessionRecording['_lazyLoadedSessionRecording']
427+
const clearBufferSpy = jest.spyOn(lazyRecorder as any, '_clearBuffer')
428+
const flushBufferSpy = jest.spyOn(lazyRecorder as any, '_flushBuffer')
429+
430+
// Trigger beforeunload
431+
window.dispatchEvent(new Event('beforeunload'))
432+
433+
// Should have cleared buffer, not flushed it
434+
expect(clearBufferSpy).toHaveBeenCalled()
435+
// Note: _flushBuffer is always called from _onBeforeUnload, but returns early if buffering
436+
expect(flushBufferSpy).toHaveBeenCalled()
437+
})
377438
})
378439
})

packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,6 +1334,13 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
13341334
}
13351335

13361336
private _onBeforeUnload = (): void => {
1337+
// If still buffering (waiting for remote config), discard the buffer
1338+
// Better to lose this data than record incorrectly.
1339+
if (this.status === BUFFERING) {
1340+
this._clearBuffer()
1341+
return
1342+
}
1343+
13371344
this._flushBuffer()
13381345
}
13391346

packages/browser/src/extensions/replay/session-recording.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ export class SessionRecording {
7373
}
7474

7575
startIfEnabledOrStop(startReason?: SessionStartReason) {
76+
// Wait for fresh remote config before starting recording
77+
// This prevents using stale cached triggers from localStorage or CDN
78+
if (!this._receivedFlags) {
79+
return
80+
}
81+
7682
if (this._isRecordingEnabled && this._lazyLoadedSessionRecording?.isStarted) {
7783
return
7884
}

0 commit comments

Comments
 (0)