🐛 [RUM-3039] Fix missing pending mutations at view end (#2598)

DataDog · Feb 19, 2024 · b9ebe46 · b9ebe46
1 parent 820faa8
commit b9ebe46
Show file tree

Hide file tree

Showing 21 changed files with 407 additions and 265 deletions.
diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts
@@ -497,6 +497,31 @@ describe('startSessionManager', () => {
         secondSessionTrackingType
       )
     })
+
+    it('should return the current session context in the renewObservable callback', () => {
+      const sessionManager = startSessionManagerWithDefaults()
+      let currentSession
+      sessionManager.renewObservable.subscribe(() => (currentSession = sessionManager.findActiveSession()))
+
+      // new session
+      expireSessionCookie()
+      document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK))
+      clock.tick(STORAGE_POLL_DELAY)
+
+      expect(currentSession).toBeDefined()
+    })
+
+    it('should return the current session context in the expireObservable callback', () => {
+      const sessionManager = startSessionManagerWithDefaults()
+      let currentSession
+      sessionManager.expireObservable.subscribe(() => (currentSession = sessionManager.findActiveSession()))
+
+      // new session
+      expireSessionCookie()
+      clock.tick(STORAGE_POLL_DELAY)
+
+      expect(currentSession).toBeDefined()
+    })
   })
 
   describe('tracking consent', () => {

diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts
@@ -1,4 +1,4 @@
-import type { Observable } from '../../tools/observable'
+import { Observable } from '../../tools/observable'
 import type { Context } from '../../tools/serialisation/context'
 import { ValueHistory } from '../../tools/valueHistory'
 import type { RelativeTime } from '../../tools/utils/timeUtils'
@@ -32,6 +32,9 @@ export function startSessionManager<TrackingType extends string>(
   computeSessionState: (rawTrackingType?: string) => { trackingType: TrackingType; isTracked: boolean },
   trackingConsentState: TrackingConsentState
 ): SessionManager<TrackingType> {
+  const renewObservable = new Observable<void>()
+  const expireObservable = new Observable<void>()
+
   // TODO - Improve configuration type and remove assertion
   const sessionStore = startSessionStore(configuration.sessionStoreStrategyType!, productKey, computeSessionState)
   stopCallbacks.push(() => sessionStore.stop())
@@ -41,8 +44,10 @@ export function startSessionManager<TrackingType extends string>(
 
   sessionStore.renewObservable.subscribe(() => {
     sessionContextHistory.add(buildSessionContext(), relativeNow())
+    renewObservable.notify()
   })
   sessionStore.expireObservable.subscribe(() => {
+    expireObservable.notify()
     sessionContextHistory.closeActive(relativeNow())
   })
 
@@ -75,8 +80,8 @@ export function startSessionManager<TrackingType extends string>(
 
   return {
     findActiveSession: (startTime) => sessionContextHistory.find(startTime),
-    renewObservable: sessionStore.renewObservable,
-    expireObservable: sessionStore.expireObservable,
+    renewObservable,
+    expireObservable,
     expire: sessionStore.expire,
   }
 }

diff --git a/packages/core/src/tools/instrumentMethod.spec.ts b/packages/core/src/tools/instrumentMethod.spec.ts
@@ -262,6 +262,20 @@ describe('instrumentSetter', () => {
       stop()
 
       object.foo = 2
+      clock.tick(0)
+
+      expect(instrumentationSetterSpy).not.toHaveBeenCalled()
+    })
+
+    it('does not call instrumentation pending in the event loop via setTimeout', () => {
+      const object = {} as { foo: number }
+      Object.defineProperty(object, 'foo', { set: noop, configurable: true })
+      const instrumentationSetterSpy = jasmine.createSpy()
+      const { stop } = instrumentSetter(object, 'foo', instrumentationSetterSpy)
+
+      object.foo = 2
+      stop()
+      clock.tick(0)
 
       expect(instrumentationSetterSpy).not.toHaveBeenCalled()
     })
@@ -284,13 +298,14 @@ describe('instrumentSetter', () => {
         const object = {} as { foo: number }
         Object.defineProperty(object, 'foo', { set: noop, configurable: true })
         const instrumentationSetterSpy = jasmine.createSpy()
-        const { stop } = instrumentSetter(object, 'foo', noop)
+        const { stop } = instrumentSetter(object, 'foo', instrumentationSetterSpy)
 
         thirdPartyInstrumentation(object)
 
         stop()
 
         object.foo = 2
+        clock.tick(0)
 
         expect(instrumentationSetterSpy).not.toHaveBeenCalled()
       })

diff --git a/packages/core/src/tools/instrumentMethod.ts b/packages/core/src/tools/instrumentMethod.ts
@@ -125,10 +125,13 @@ export function instrumentSetter<TARGET extends { [key: string]: any }, PROPERTY
     return { stop: noop }
   }
 
+  const stoppedInstrumentation = noop
   let instrumentation = (target: TARGET, value: TARGET[PROPERTY]) => {
     // put hooked setter into event loop to avoid of set latency
     setTimeout(() => {
-      after(target, value)
+      if (instrumentation !== stoppedInstrumentation) {
+        after(target, value)
+      }
     }, 0)
   }
 
@@ -145,9 +148,8 @@ export function instrumentSetter<TARGET extends { [key: string]: any }, PROPERTY
     stop: () => {
       if (Object.getOwnPropertyDescriptor(targetPrototype, property)?.set === instrumentationWrapper) {
         Object.defineProperty(targetPrototype, property, originalDescriptor)
-      } else {
-        instrumentation = noop
       }
+      instrumentation = stoppedInstrumentation
     },
   }
 }
diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts
@@ -43,7 +43,7 @@ describe('featureFlagContexts', () => {
 
       const { lifeCycle } = setupBuilder.build()
 
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(0 as RelativeTime),
       } as ViewCreatedEvent)
 
@@ -67,7 +67,7 @@ describe('featureFlagContexts', () => {
 
       const { lifeCycle } = setupBuilder.build()
 
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(0 as RelativeTime),
       } as ViewCreatedEvent)
 
@@ -83,7 +83,7 @@ describe('featureFlagContexts', () => {
     it('should not add feature flag evaluation when the ff feature_flags is disabled', () => {
       const { lifeCycle } = setupBuilder.build()
 
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(0 as RelativeTime),
       } as ViewCreatedEvent)
 
@@ -101,7 +101,7 @@ describe('featureFlagContexts', () => {
 
       const updateCustomerDataSpy = spyOn(customerDataTracker, 'updateCustomerData')
 
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(0 as RelativeTime),
       } as ViewCreatedEvent)
 
@@ -129,14 +129,14 @@ describe('featureFlagContexts', () => {
 
       const { lifeCycle } = setupBuilder.build()
 
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(0 as RelativeTime),
       } as ViewCreatedEvent)
       featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo')
-      lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
+      lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
         endClocks: relativeToClocks(10 as RelativeTime),
       } as ViewEndedEvent)
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(10 as RelativeTime),
       } as ViewCreatedEvent)
 
@@ -149,16 +149,16 @@ describe('featureFlagContexts', () => {
 
       const { lifeCycle, clock } = setupBuilder.withFakeClock().build()
 
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(0 as RelativeTime),
       } as ViewCreatedEvent)
 
       clock.tick(10)
       featureFlagContexts.addFeatureFlagEvaluation('feature', 'one')
-      lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
+      lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
         endClocks: relativeToClocks(10 as RelativeTime),
       } as ViewEndedEvent)
-      lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+      lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
         startClocks: relativeToClocks(10 as RelativeTime),
       } as ViewCreatedEvent)
 

diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.ts
@@ -42,15 +42,15 @@ export function startFeatureFlagContexts(
 
   const featureFlagContexts = new ValueHistory<FeatureFlagContext>(FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY)
 
-  lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
-    featureFlagContexts.closeActive(endClocks.relative)
-  })
-
-  lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => {
+  lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, ({ startClocks }) => {
     featureFlagContexts.add({}, startClocks.relative)
     customerDataTracker.resetCustomerData()
   })
 
+  lifeCycle.subscribe(LifeCycleEventType.AFTER_VIEW_ENDED, ({ endClocks }) => {
+    featureFlagContexts.closeActive(endClocks.relative)
+  })
+
   return {
     findFeatureFlagEvaluations: (startTime?: RelativeTime) => featureFlagContexts.find(startTime),
     addFeatureFlagEvaluation: (key: string, value: ContextValue) => {

diff --git a/packages/rum-core/src/domain/contexts/urlContexts.spec.ts b/packages/rum-core/src/domain/contexts/urlContexts.spec.ts
@@ -38,7 +38,7 @@ describe('urlContexts', () => {
   it('should return current url and document referrer for initial view', () => {
     const { lifeCycle } = setupBuilder.build()
 
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(0 as RelativeTime),
     } as ViewCreatedEvent)
 
@@ -50,7 +50,7 @@ describe('urlContexts', () => {
   it('should update url context on location change', () => {
     const { lifeCycle, changeLocation } = setupBuilder.build()
 
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(0 as RelativeTime),
     } as ViewCreatedEvent)
     changeLocation('/foo')
@@ -63,14 +63,14 @@ describe('urlContexts', () => {
   it('should update url context on new view', () => {
     const { lifeCycle, changeLocation } = setupBuilder.build()
 
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(0 as RelativeTime),
     } as ViewCreatedEvent)
     changeLocation('/foo')
-    lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
+    lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
       endClocks: relativeToClocks(10 as RelativeTime),
     } as ViewEndedEvent)
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(10 as RelativeTime),
     } as ViewCreatedEvent)
 
@@ -82,16 +82,16 @@ describe('urlContexts', () => {
   it('should return the url context corresponding to the start time', () => {
     const { lifeCycle, changeLocation, clock } = setupBuilder.build()
 
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(0 as RelativeTime),
     } as ViewCreatedEvent)
 
     clock.tick(10)
     changeLocation('/foo')
-    lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
+    lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
       endClocks: relativeToClocks(10 as RelativeTime),
     } as ViewEndedEvent)
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(10 as RelativeTime),
     } as ViewCreatedEvent)
 
@@ -100,10 +100,10 @@ describe('urlContexts', () => {
 
     clock.tick(10)
     changeLocation('/qux')
-    lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
+    lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
       endClocks: relativeToClocks(30 as RelativeTime),
     } as ViewEndedEvent)
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(30 as RelativeTime),
     } as ViewCreatedEvent)
 
@@ -132,10 +132,10 @@ describe('urlContexts', () => {
   it('should return undefined when no current view', () => {
     const { lifeCycle } = setupBuilder.build()
 
-    lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
+    lifeCycle.notify(LifeCycleEventType.BEFORE_VIEW_CREATED, {
       startClocks: relativeToClocks(0 as RelativeTime),
     } as ViewCreatedEvent)
-    lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
+    lifeCycle.notify(LifeCycleEventType.AFTER_VIEW_ENDED, {
       endClocks: relativeToClocks(10 as RelativeTime),
     } as ViewEndedEvent)
 

diff --git a/packages/rum-core/src/domain/contexts/urlContexts.ts b/packages/rum-core/src/domain/contexts/urlContexts.ts
@@ -31,11 +31,7 @@ export function startUrlContexts(
 
   let previousViewUrl: string | undefined
 
-  lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
-    urlContextHistory.closeActive(endClocks.relative)
-  })
-
-  lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => {
+  lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, ({ startClocks }) => {
     const viewUrl = location.href
     urlContextHistory.add(
       buildUrlContext({
@@ -47,6 +43,10 @@ export function startUrlContexts(
     previousViewUrl = viewUrl
   })
 
+  lifeCycle.subscribe(LifeCycleEventType.AFTER_VIEW_ENDED, ({ endClocks }) => {
+    urlContextHistory.closeActive(endClocks.relative)
+  })
+
   const locationChangeSubscription = locationChangeObservable.subscribe(({ newLocation }) => {
     const current = urlContextHistory.find()
     if (current) {