🐛 ignore full snapshots taken before "load" event

The recorder starts recording at the document "load" event.

If a view changes before the "load" event, a full snapshot is taken,
then another one is taken at the "load" event, resulting in two
consecutive full snapshots on the same view.

This commit fixes this issue by ignoring any full snapshot occurring
before the actual recording start, and makes sure early events have the
correct `has_replay` value.

packages/rum-recorder/src/boot/recorder.ts

@@ -25,7 +25,7 @@ export function startRecording(
     addRecord({ ...rawRecord, timestamp: Date.now() })
   }
 
-  const { stop: stopRecording, takeFullSnapshot } = record({
+  const { stop: stopRecording, takeFullSnapshot, isRecording } = record({
     emit: addRawRecord,
   })
 
@@ -37,6 +37,7 @@ export function startRecording(
       stopRecording()
       stopSegmentCollection()
     },
+    isRecording,
   }
 }
 

packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts

@@ -10,11 +10,14 @@ describe('makeRumRecorderPublicApi', () => {
   let startRecordingSpy: jasmine.Spy<StartRecording>
   let stopRecordingSpy: jasmine.Spy<() => void>
   let startRumSpy: jasmine.Spy<StartRum>
+  let isRecording: boolean
 
   beforeEach(() => {
+    isRecording = true
     stopRecordingSpy = jasmine.createSpy()
     startRecordingSpy = jasmine.createSpy().and.callFake(() => ({
       stop: stopRecordingSpy,
+      isRecording: () => isRecording,
     }))
     startRumSpy = jasmine.createSpy<StartRum>().and.callFake((userConfiguration) => {
       const configuration: Partial<Configuration> = {
@@ -92,9 +95,12 @@ describe('makeRumRecorderPublicApi', () => {
 
   describe('commonContext hasReplay', () => {
     it('is true only if recording', () => {
+      isRecording = false
       rumRecorderPublicApi.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
       expect(getCommonContext().hasReplay).toBeUndefined()
       rumRecorderPublicApi.startSessionReplayRecording()
+      expect(getCommonContext().hasReplay).toBeUndefined()
+      isRecording = true
       expect(getCommonContext().hasReplay).toBe(true)
       rumRecorderPublicApi.stopSessionReplayRecording()
       expect(getCommonContext().hasReplay).toBeUndefined()

packages/rum-recorder/src/boot/rumRecorderPublicApi.ts

@@ -21,6 +21,7 @@ type RecorderState =
   | {
       status: RecorderStatus.Started
       stopRecording: () => void
+      isRecording: () => void
     }
 
 export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) {
@@ -31,7 +32,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI
 
     const startRumResult = startRumImpl(userConfiguration, () => ({
       ...getCommonContext(),
-      hasReplay: state.status === RecorderStatus.Started ? true : undefined,
+      hasReplay: state.status === RecorderStatus.Started && state.isRecording() ? true : undefined,
     }))
 
     const { lifeCycle, parentContexts, configuration, session } = startRumResult
@@ -41,7 +42,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI
         return
       }
 
-      const { stop: stopRecording } = startRecordingImpl(
+      const { stop: stopRecording, isRecording } = startRecordingImpl(
         lifeCycle,
         userConfiguration.applicationId,
         configuration,
@@ -51,6 +52,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI
       state = {
         status: RecorderStatus.Started,
         stopRecording,
+        isRecording,
       }
       lifeCycle.notify(LifeCycleEventType.RECORD_STARTED)
     }

packages/rum-recorder/src/domain/record/record.spec.ts

@@ -28,11 +28,19 @@ describe('record', () => {
     recordApi?.stop()
   })
 
+  it('does not start recording before the page "onload"', () => {
+    const { triggerOnLoad } = mockDocumentReadyState()
+    initRecording()
+    expect(recordApi.isRecording()).toBe(false)
+    triggerOnLoad()
+    expect(recordApi.isRecording()).toBe(true)
+  })
+
   it('captures stylesheet rules', (done) => {
     const styleElement = document.createElement('style')
     sandbox.appendChild(styleElement)
 
-    startRecording()
+    initRecording()
 
     const styleSheet = styleElement.sheet as CSSStyleSheet
     const ruleIdx0 = styleSheet.insertRule('body { background: #000; }')
@@ -101,26 +109,35 @@ describe('record', () => {
     })
   })
 
-  it('flushes pending mutation records before taking a full snapshot', (done) => {
-    startRecording()
+  describe('takeFullSnapshot', () => {
+    it('ignores any "takeFullSnapshot" call while it is not recording', () => {
+      mockDocumentReadyState()
+      initRecording()
+      recordApi.takeFullSnapshot()
+      expect(emitSpy).not.toHaveBeenCalled()
+    })
 
-    sandbox.appendChild(document.createElement('div'))
+    it('flushes pending mutation records before taking a full snapshot', (done) => {
+      initRecording()
 
-    recordApi.takeFullSnapshot()
+      sandbox.appendChild(document.createElement('div'))
 
-    waitEmitCalls(7, () => {
-      const records = getEmittedRecords()
+      recordApi.takeFullSnapshot()
 
-      expect(records[0].type).toEqual(RecordType.Meta)
-      expect(records[1].type).toEqual(RecordType.Focus)
-      expect(records[2].type).toEqual(RecordType.FullSnapshot)
-      expect(records[3].type).toEqual(RecordType.IncrementalSnapshot)
-      expect((records[3] as IncrementalSnapshotRecord).data.source).toEqual(IncrementalSource.Mutation)
-      expect(records[4].type).toEqual(RecordType.Meta)
-      expect(records[5].type).toEqual(RecordType.Focus)
-      expect(records[6].type).toEqual(RecordType.FullSnapshot)
+      waitEmitCalls(7, () => {
+        const records = getEmittedRecords()
 
-      expectNoExtraEmitCalls(done)
+        expect(records[0].type).toEqual(RecordType.Meta)
+        expect(records[1].type).toEqual(RecordType.Focus)
+        expect(records[2].type).toEqual(RecordType.FullSnapshot)
+        expect(records[3].type).toEqual(RecordType.IncrementalSnapshot)
+        expect((records[3] as IncrementalSnapshotRecord).data.source).toEqual(IncrementalSource.Mutation)
+        expect(records[4].type).toEqual(RecordType.Meta)
+        expect(records[5].type).toEqual(RecordType.Focus)
+        expect(records[6].type).toEqual(RecordType.FullSnapshot)
+
+        expectNoExtraEmitCalls(done)
+      })
     })
   })
 
@@ -133,7 +150,7 @@ describe('record', () => {
     })
 
     it('adds an initial Focus record when starting to record', () => {
-      startRecording()
+      initRecording()
       expect(getEmittedRecords()[1]).toEqual({
         type: RecordType.Focus,
         data: {
@@ -143,23 +160,23 @@ describe('record', () => {
     })
 
     it('adds a Focus record on focus', () => {
-      startRecording()
+      initRecording()
       emitSpy.calls.reset()
 
       window.dispatchEvent(createNewEvent('focus'))
       expect(getEmittedRecords()[0].type).toBe(RecordType.Focus)
     })
 
     it('adds a Focus record on blur', () => {
-      startRecording()
+      initRecording()
       emitSpy.calls.reset()
 
       window.dispatchEvent(createNewEvent('blur'))
       expect(getEmittedRecords()[0].type).toBe(RecordType.Focus)
     })
 
     it('adds a Focus record on when taking a full snapshot', () => {
-      startRecording()
+      initRecording()
       emitSpy.calls.reset()
 
       recordApi.takeFullSnapshot()
@@ -168,18 +185,18 @@ describe('record', () => {
 
     it('set has_focus to true if the document has the focus', () => {
       hasFocus = true
-      startRecording()
+      initRecording()
       expect((getEmittedRecords()[1] as FocusRecord).data.has_focus).toBe(true)
     })
 
     it("set has_focus to false if the document doesn't have the focus", () => {
       hasFocus = false
-      startRecording()
+      initRecording()
       expect((getEmittedRecords()[1] as FocusRecord).data.has_focus).toBe(false)
     })
   })
 
-  function startRecording() {
+  function initRecording() {
     recordApi = record({
       emit: emitSpy,
     })
@@ -196,3 +213,14 @@ function createDOMSandbox() {
   document.body.appendChild(sandbox)
   return sandbox
 }
+
+function mockDocumentReadyState() {
+  let readyState = 'loading'
+  spyOnProperty(Document.prototype, 'readyState', 'get').and.callFake(() => readyState)
+  return {
+    triggerOnLoad: () => {
+      readyState = 'complete'
+      window.dispatchEvent(createNewEvent('load'))
+    },
+  }
+}

packages/rum-recorder/src/domain/record/record.ts

@@ -13,9 +13,13 @@ export function record(options: RecordOptions): RecordAPI {
     throw new Error('emit function is required')
   }
 
+  let isRecording = false
   const mutationController = new MutationController()
 
   const takeFullSnapshot = () => {
+    if (!isRecording) {
+      return
+    }
     mutationController.flush() // process any pending mutation before taking a full snapshot
 
     emit({
@@ -59,7 +63,8 @@ export function record(options: RecordOptions): RecordAPI {
   }
 
   const handlers: ListenerHandler[] = []
-  const init = () => {
+  const startRecording = () => {
+    isRecording = true
     takeFullSnapshot()
 
     handlers.push(
@@ -138,12 +143,14 @@ export function record(options: RecordOptions): RecordAPI {
     )
   }
 
-  runOnReadyState('complete', init)
+  runOnReadyState('complete', startRecording)
 
   return {
     stop: () => {
+      isRecording = false
       handlers.forEach((h) => h())
     },
     takeFullSnapshot,
+    isRecording: () => isRecording,
   }
 }

packages/rum-recorder/src/domain/record/types.ts

@@ -66,6 +66,7 @@ export interface RecordOptions {
 export interface RecordAPI {
   stop: ListenerHandler
   takeFullSnapshot: () => void
+  isRecording: () => boolean
 }
 
 export interface ObserverParam {