♻️ [RUMF-1505] introduce a safe setInterval helper function

eslint-local-rules/disallowZoneJsPatchedValues.js

@@ -8,6 +8,11 @@ const PROBLEMATIC_IDENTIFIERS = {
   setTimeout: 'Use `setTimeout` from @datadog/browser-core instead',
   clearTimeout: 'Use `clearTimeout` from @datadog/browser-core instead',
 
+  // We didn't stumble on cases where using the patched `setInterval` from Zone.js is problematic
+  // yet, but still consider it problematic in prevention and to unify its usages with `setTimeout`.
+  setInterval: 'Use `setInterval` from @datadog/browser-core instead',
+  clearInterval: 'Use `clearInterval` from @datadog/browser-core instead',
+
   // TODO: disallow addEventListener, removeEventListener
 }
 

packages/core/src/browser/timer.spec.ts

@@ -3,66 +3,78 @@ import type { Clock } from '../../test/specHelper'
 import { mockClock } from '../../test/specHelper'
 import { noop } from '../tools/utils'
 import { resetMonitor, startMonitorErrorCollection } from '../tools/monitor'
-import { setTimeout, clearTimeout } from './timer'
+import { setTimeout, clearTimeout, setInterval, clearInterval } from './timer'
+;[
+  {
+    name: 'setTimeout' as const,
+    setTimer: setTimeout,
+    clearTimer: clearTimeout,
+  },
+  {
+    name: 'setInterval' as const,
+    setTimer: setInterval,
+    clearTimer: clearInterval,
+  },
+].forEach(({ name, setTimer, clearTimer }) => {
+  describe(name, () => {
+    let clock: Clock
+    let zoneJsStub: ReturnType<typeof stubZoneJs>
 
-describe('setTimeout', () => {
-  let clock: Clock
-  let zoneJsStub: ReturnType<typeof stubZoneJs>
+    beforeEach(() => {
+      clock = mockClock()
+      zoneJsStub = stubZoneJs()
+    })
 
-  beforeEach(() => {
-    clock = mockClock()
-    zoneJsStub = stubZoneJs()
-  })
+    afterEach(() => {
+      zoneJsStub.restore()
+      clock.cleanup()
+      resetMonitor()
+    })
 
-  afterEach(() => {
-    zoneJsStub.restore()
-    clock.cleanup()
-    resetMonitor()
-  })
+    it('executes the callback asynchronously', () => {
+      const spy = jasmine.createSpy()
+      setTimer(spy)
+      expect(spy).not.toHaveBeenCalled()
+      clock.tick(0)
+      expect(spy).toHaveBeenCalledOnceWith()
+    })
 
-  it('executes the callback asynchronously', () => {
-    const spy = jasmine.createSpy()
-    setTimeout(spy)
-    expect(spy).not.toHaveBeenCalled()
-    clock.tick(0)
-    expect(spy).toHaveBeenCalledOnceWith()
-  })
+    it('schedules an asynchronous task', () => {
+      const spy = jasmine.createSpy()
+      setTimer(spy)
+      expect(spy).not.toHaveBeenCalled()
+      clock.tick(0)
+      expect(spy).toHaveBeenCalledOnceWith()
+    })
 
-  it('schedules an timeout task', () => {
-    const spy = jasmine.createSpy()
-    setTimeout(spy)
-    expect(spy).not.toHaveBeenCalled()
-    clock.tick(0)
-    expect(spy).toHaveBeenCalledOnceWith()
-  })
+    it('does not use the Zone.js function', () => {
+      const zoneJsSetTimerSpy = jasmine.createSpy()
+      zoneJsStub.replaceProperty(window, name, zoneJsSetTimerSpy)
 
-  it('does not use the Zone.js setTimeout function', () => {
-    const zoneJsSetTimeoutSpy = jasmine.createSpy()
-    zoneJsStub.replaceProperty(window, 'setTimeout', zoneJsSetTimeoutSpy)
+      setTimer(noop)
+      clock.tick(0)
 
-    setTimeout(noop)
-    clock.tick(0)
+      expect(zoneJsSetTimerSpy).not.toHaveBeenCalled()
+    })
 
-    expect(zoneJsSetTimeoutSpy).not.toHaveBeenCalled()
-  })
+    it('monitors the callback', () => {
+      const onMonitorErrorCollectedSpy = jasmine.createSpy()
+      startMonitorErrorCollection(onMonitorErrorCollectedSpy)
 
-  it('monitors the callback', () => {
-    const onMonitorErrorCollectedSpy = jasmine.createSpy()
-    startMonitorErrorCollection(onMonitorErrorCollectedSpy)
+      setTimer(() => {
+        throw new Error('foo')
+      })
+      clock.tick(0)
 
-    setTimeout(() => {
-      throw new Error('foo')
+      expect(onMonitorErrorCollectedSpy).toHaveBeenCalledOnceWith(new Error('foo'))
     })
-    clock.tick(0)
 
-    expect(onMonitorErrorCollectedSpy).toHaveBeenCalledOnceWith(new Error('foo'))
-  })
-
-  it('can be canceled by using `clearTimeout`', () => {
-    const spy = jasmine.createSpy()
-    const timeoutId = setTimeout(spy)
-    clearTimeout(timeoutId)
-    clock.tick(0)
-    expect(spy).not.toHaveBeenCalled()
+    it('can be canceled', () => {
+      const spy = jasmine.createSpy()
+      const timerId = setTimer(spy)
+      clearTimer(timerId)
+      clock.tick(0)
+      expect(spy).not.toHaveBeenCalled()
+    })
   })
 })

packages/core/src/browser/timer.ts

@@ -10,3 +10,11 @@ export function setTimeout(callback: () => void, delay?: number): TimeoutId {
 export function clearTimeout(timeoutId: TimeoutId | undefined) {
   getZoneJsOriginalValue(window, 'clearTimeout')(timeoutId)
 }
+
+export function setInterval(callback: () => void, delay?: number): TimeoutId {
+  return getZoneJsOriginalValue(window, 'setInterval')(monitor(callback), delay)
+}
+
+export function clearInterval(timeoutId: TimeoutId | undefined) {
+  getZoneJsOriginalValue(window, 'clearInterval')(timeoutId)
+}

packages/core/src/domain/session/sessionManager.ts

@@ -5,8 +5,8 @@ import type { Context } from '../../tools/context'
 import { ContextHistory } from '../../tools/contextHistory'
 import type { RelativeTime } from '../../tools/timeUtils'
 import { relativeNow, clocksOrigin } from '../../tools/timeUtils'
-import { monitor } from '../../tools/monitor'
 import { DOM_EVENT, addEventListener, addEventListeners } from '../../browser/addEventListener'
+import { clearInterval, setInterval } from '../../browser/timer'
 import { tryOldCookiesMigration } from './oldCookiesMigration'
 import { startSessionStore } from './sessionStore'
 import { SESSION_TIME_OUT_DELAY } from './sessionConstants'
@@ -81,11 +81,11 @@ function trackActivity(expandOrRenewSession: () => void) {
 }
 
 function trackVisibility(expandSession: () => void) {
-  const expandSessionWhenVisible = monitor(() => {
+  const expandSessionWhenVisible = () => {
     if (document.visibilityState === 'visible') {
       expandSession()
     }
-  })
+  }
 
   const { stop } = addEventListener(document, DOM_EVENT.VISIBILITY_CHANGE, expandSessionWhenVisible)
   stopCallbacks.push(stop)

packages/core/src/domain/session/sessionStore.ts

@@ -1,6 +1,6 @@
 import type { CookieOptions } from '../../browser/cookie'
 import { COOKIE_ACCESS_DELAY } from '../../browser/cookie'
-import { monitor } from '../../tools/monitor'
+import { clearInterval, setInterval } from '../../browser/timer'
 import { Observable } from '../../tools/observable'
 import { dateNow } from '../../tools/timeUtils'
 import * as utils from '../../tools/utils'
@@ -39,7 +39,7 @@ export function startSessionStore<TrackingType extends string>(
   const renewObservable = new Observable<void>()
   const expireObservable = new Observable<void>()
 
-  const watchSessionTimeoutId = setInterval(monitor(watchSession), COOKIE_ACCESS_DELAY)
+  const watchSessionTimeoutId = setInterval(watchSession, COOKIE_ACCESS_DELAY)
   let sessionCache: SessionState = retrieveActiveSession()
 
   function expandOrRenewSession() {

packages/core/src/tools/contextHistory.ts

@@ -1,3 +1,4 @@
+import { setInterval, clearInterval } from '../browser/timer'
 import type { TimeoutId } from '../browser/timer'
 import type { RelativeTime } from './timeUtils'
 import { relativeNow } from './timeUtils'

packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts

@@ -5,14 +5,15 @@ import {
   assign,
   elapsed,
   generateUUID,
-  monitor,
   ONE_MINUTE,
   throttle,
   clocksNow,
   clocksOrigin,
   timeStampNow,
   display,
   looksLikeRelativeTime,
+  setInterval,
+  clearInterval,
 } from '@datadog/browser-core'
 
 import type { ViewCustomTimings } from '../../../rawRumEvent.types'
@@ -136,12 +137,9 @@ export function trackViews(
     })
 
     // Session keep alive
-    const keepAliveInterval = window.setInterval(
-      monitor(() => {
-        currentView.triggerUpdate()
-      }),
-      SESSION_KEEP_ALIVE_INTERVAL
-    )
+    const keepAliveInterval = setInterval(() => {
+      currentView.triggerUpdate()
+    }, SESSION_KEEP_ALIVE_INTERVAL)
 
     return {
       stop: () => {

packages/rum-core/src/domain/startCustomerDataTelemetry.ts

@@ -1,5 +1,5 @@
 import type { BatchFlushEvent, Context, ContextManager, Observable, Telemetry } from '@datadog/browser-core'
-import { isEmptyObject, includes, performDraw, ONE_SECOND, addTelemetryDebug, monitor } from '@datadog/browser-core'
+import { isEmptyObject, includes, performDraw, ONE_SECOND, addTelemetryDebug, setInterval } from '@datadog/browser-core'
 import { RumEventType } from '../rawRumEvent.types'
 import type { RumEvent } from '../rumEvent.types'
 import type { RumConfiguration } from './configuration'
@@ -91,7 +91,7 @@ export function startCustomerDataTelemetry(
     initCurrentBatchMeasures()
   })
 
-  setInterval(monitor(sendCurrentPeriodMeasures), MEASURES_PERIOD_DURATION)
+  setInterval(sendCurrentPeriodMeasures, MEASURES_PERIOD_DURATION)
 }
 
 function sendCurrentPeriodMeasures() {