🐛 [Shopify] Defer plugin init until a checkout page view, fixing double session IDs

packages/browser-core/src/index.ts

@@ -142,6 +142,7 @@ export * from './browser/addEventListener'
 export { requestIdleCallback } from './tools/requestIdleCallback'
 export * from './tools/taskQueue'
 export * from './tools/timer'
+export * from './tools/thenable'
 export type { ConsoleLog } from './domain/console/consoleObservable'
 export { initConsoleObservable } from './domain/console/consoleObservable'
 export { catchUserErrors } from './tools/catchUserErrors'

packages/browser-core/src/tools/thenable.spec.ts

@@ -0,0 +1,42 @@
+import { isThenable, waitForThenable, TIMEOUT_ERROR_MESSAGE } from './thenable'
+import { noop } from './utils/functionUtils'
+
+describe('isThenable', () => {
+  it('returns true for a Promise', () => {
+    expect(isThenable(Promise.resolve())).toBe(true)
+  })
+
+  it('returns true for a plain object with a then function', () => {
+    expect(isThenable({ then: noop })).toBe(true)
+  })
+
+  it('returns false for a plain object without a then function', () => {
+    expect(isThenable({})).toBe(false)
+  })
+
+  it('returns false for null and undefined', () => {
+    expect(isThenable(null)).toBe(false)
+    expect(isThenable(undefined)).toBe(false)
+  })
+
+  it('returns false for primitives', () => {
+    expect(isThenable(42)).toBe(false)
+    expect(isThenable('foo')).toBe(false)
+  })
+})
+
+describe('waitForThenable', () => {
+  it('resolves with the thenable value when it settles before the timeout', async () => {
+    const result = await waitForThenable(Promise.resolve('value'), 1000)
+    expect(result).toBe('value')
+  })
+
+  it('rejects with the thenable rejection reason when it settles before the timeout', async () => {
+    await expectAsync(waitForThenable(Promise.reject(new Error('boom')), 1000)).toBeRejectedWithError('boom')
+  })
+
+  it('rejects with a timeout error when the thenable does not settle in time', async () => {
+    const neverSettles = new Promise(noop)
+    await expectAsync(waitForThenable(neverSettles, 0)).toBeRejectedWithError(TIMEOUT_ERROR_MESSAGE)
+  })
+})

packages/browser-core/src/tools/thenable.ts

@@ -0,0 +1,23 @@
+import type { TimeoutId } from './timer'
+import { setTimeout, clearTimeout } from './timer'
+
+export function isThenable<T>(value: unknown): value is PromiseLike<T> {
+  return !!value && typeof (value as { then?: unknown }).then === 'function'
+}
+
+export const TIMEOUT_ERROR_MESSAGE = 'Timeout'
+export function isTimeoutError(error: unknown): error is Error {
+  return error instanceof Error && error.message === TIMEOUT_ERROR_MESSAGE
+}
+
+/**
+ * Resolves or rejects with `thenable`, or rejects with a `TIMEOUT_ERROR_MESSAGE` error if it
+ * doesn't settle within `timeout` ms.
+ */
+export function waitForThenable<T>(thenable: PromiseLike<T>, timeout = 3000): Promise<T> {
+  let timeoutId: TimeoutId
+  const timeoutPromise = new Promise<never>((_, reject) => {
+    timeoutId = setTimeout(() => reject(new Error(TIMEOUT_ERROR_MESSAGE)), timeout)
+  })
+  return Promise.race([thenable, timeoutPromise]).finally(() => clearTimeout(timeoutId))
+}

packages/browser-rum-angular/src/domain/angularPlugin.spec.ts

@@ -32,7 +32,7 @@ describe('angularPlugin', () => {
 
     expect(callbackSpy).not.toHaveBeenCalled()
 
-    angularPlugin(pluginConfiguration).onInit!({
+    void angularPlugin(pluginConfiguration).onInit!({
       publicApi: PUBLIC_API,
       initConfiguration: INIT_CONFIGURATION,
     })
@@ -45,7 +45,7 @@ describe('angularPlugin', () => {
   it('calls callbacks immediately if onInit was already invoked', () => {
     const callbackSpy = jasmine.createSpy()
     const pluginConfiguration = {}
-    angularPlugin(pluginConfiguration).onInit!({
+    void angularPlugin(pluginConfiguration).onInit!({
       publicApi: PUBLIC_API,
       initConfiguration: INIT_CONFIGURATION,
     })
@@ -59,14 +59,14 @@ describe('angularPlugin', () => {
 
   it('enforce manual view tracking when router is enabled', () => {
     const initConfiguration = { ...INIT_CONFIGURATION }
-    angularPlugin({ router: true }).onInit!({ publicApi: PUBLIC_API, initConfiguration })
+    void angularPlugin({ router: true }).onInit!({ publicApi: PUBLIC_API, initConfiguration })
 
     expect(initConfiguration.trackViewsManually).toBe(true)
   })
 
   it('does not enforce manual view tracking when router is disabled', () => {
     const initConfiguration = { ...INIT_CONFIGURATION }
-    angularPlugin({ router: false }).onInit!({ publicApi: PUBLIC_API, initConfiguration })
+    void angularPlugin({ router: false }).onInit!({ publicApi: PUBLIC_API, initConfiguration })
 
     expect(initConfiguration.trackViewsManually).toBeUndefined()
   })

packages/browser-rum-angular/test/initializeAngularPlugin.ts

@@ -11,7 +11,7 @@ export function initializeAngularPlugin({
   resetAngularPlugin()
   const plugin = angularPlugin()
 
-  plugin.onInit!({
+  void plugin.onInit!({
     publicApi: {} as RumPublicApi,
     initConfiguration: {} as RumInitConfiguration,
   })

packages/browser-rum-core/src/boot/preStartRum.spec.ts

@@ -620,6 +620,45 @@ describe('preStartRum', () => {
         expect(doStartRumSpy).toHaveBeenCalled()
         expect(doStartRumSpy.calls.mostRecent().args[0].applicationId).toBe('application-id')
       })
+
+      it('does not start RUM, synchronously, when a plugin synchronously returns false', () => {
+        const plugin: RumPlugin = { name: 'a', onInit: () => false }
+        const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
+
+        strategy.init({ ...DEFAULT_INIT_CONFIGURATION, plugins: [plugin] }, PUBLIC_API)
+
+        expect(doStartRumSpy).not.toHaveBeenCalled()
+      })
+
+      it('starts RUM once a plugin resolves its onInit promise to void', async () => {
+        const plugin: RumPlugin = { name: 'a', onInit: () => Promise.resolve() }
+        const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
+
+        strategy.init({ ...DEFAULT_INIT_CONFIGURATION, plugins: [plugin] }, PUBLIC_API)
+        await collectAsyncCalls(doStartRumSpy, 1)
+
+        expect(doStartRumSpy).toHaveBeenCalled()
+      })
+
+      it('does not start RUM when a plugin resolves its onInit promise to false', async () => {
+        const plugin: RumPlugin = { name: 'a', onInit: () => Promise.resolve(false) }
+        const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
+
+        strategy.init({ ...DEFAULT_INIT_CONFIGURATION, plugins: [plugin] }, PUBLIC_API)
+        await new Promise((resolve) => setTimeout(resolve))
+
+        expect(doStartRumSpy).not.toHaveBeenCalled()
+      })
+
+      it('does not start RUM and does not throw when a plugin onInit promise rejects', async () => {
+        const plugin: RumPlugin = { name: 'a', onInit: () => Promise.reject(new Error('boom')) }
+        const { strategy, doStartRumSpy } = createPreStartStrategyWithDefaults()
+
+        strategy.init({ ...DEFAULT_INIT_CONFIGURATION, plugins: [plugin] }, PUBLIC_API)
+        await new Promise((resolve) => setTimeout(resolve))
+
+        expect(doStartRumSpy).not.toHaveBeenCalled()
+      })
     })
   })
 

packages/browser-rum-core/src/boot/preStartRum.ts

@@ -42,7 +42,7 @@ import {
 } from '../domain/configuration'
 import type { ViewOptions } from '../domain/view/trackViews'
 import type { OperationOptions, FailureReason } from '../domain/vital/vitalCollection'
-import { callPluginsMethod } from '../domain/plugins'
+import { runOnInitPlugins } from '../domain/plugins'
 import { startTrackingConsentContext } from '../domain/contexts/trackingConsentContext'
 import type { StartRumResult } from './startRum'
 import type { RumPublicApiOptions, Strategy } from './rumPublicApi'
@@ -242,31 +242,48 @@ export function createPreStartStrategy(
         return
       }
 
-      callPluginsMethod(initConfiguration.plugins, 'onInit', { initConfiguration, publicApi })
-
-      const hasRemoteConfiguration = getRemoteConfigurationId(initConfiguration)
-
-      if (hasRemoteConfiguration) {
-        const supportedContextManagers = { user: userContext, context: globalContext }
-        const isSyncLoading = !!initConfiguration.remoteConfigurationId || !!initConfiguration.remoteConfiguration?.sync
-
-        if (isSyncLoading) {
-          fetchAndApplyRemoteConfiguration(initConfiguration, supportedContextManagers)
-            .then((resolvedInitConfiguration) => {
-              if (resolvedInitConfiguration) {
-                doInit(resolvedInitConfiguration, errorStack)
-              }
-            })
-            .catch(monitorError)
+      function proceedWithInit() {
+        const hasRemoteConfiguration = getRemoteConfigurationId(initConfiguration)
+
+        if (hasRemoteConfiguration) {
+          const supportedContextManagers = { user: userContext, context: globalContext }
+          const isSyncLoading =
+            !!initConfiguration.remoteConfigurationId || !!initConfiguration.remoteConfiguration?.sync
+
+          if (isSyncLoading) {
+            fetchAndApplyRemoteConfiguration(initConfiguration, supportedContextManagers)
+              .then((resolvedInitConfiguration) => {
+                if (resolvedInitConfiguration) {
+                  doInit(resolvedInitConfiguration, errorStack)
+                }
+              })
+              .catch(monitorError)
+          } else {
+            const resolvedInitConfiguration = getRemoteConfiguration(initConfiguration, supportedContextManagers)
+
+            if (resolvedInitConfiguration) {
+              doInit(resolvedInitConfiguration, errorStack)
+            }
+          }
         } else {
-          const resolvedInitConfiguration = getRemoteConfiguration(initConfiguration, supportedContextManagers)
+          doInit(initConfiguration, errorStack)
+        }
+      }
 
-          if (resolvedInitConfiguration) {
-            doInit(resolvedInitConfiguration, errorStack)
-          }
+      const shouldContinue = runOnInitPlugins(initConfiguration.plugins, { initConfiguration, publicApi })
+
+      if (typeof shouldContinue === 'boolean') {
+        if (shouldContinue) {
+          proceedWithInit()
         }
       } else {
-        doInit(initConfiguration, errorStack)
+        shouldContinue
+          .then((result) => {
+            if (result) {
+              proceedWithInit()
+            }
+          })
+          .catch(monitorError)
       }
     },
 

packages/browser-rum-core/src/boot/rumPublicApi.ts

@@ -50,7 +50,8 @@ import type {
   FeatureOperationOptions,
   FailureReason,
 } from '../domain/vital/vitalCollection'
-import { callPluginsMethod } from '../domain/plugins'
+import type { RumPlugin } from '../domain/plugins'
+import { callPluginsOnRumStart } from '../domain/plugins'
 import type { Hooks } from '../domain/hooks'
 import type { SdkName } from '../domain/contexts/defaultContext'
 import type { ActionOptions } from '../domain/action/trackManualActions'
@@ -622,6 +623,8 @@ export interface ProfilerApi {
   ) => void
 }
 
+export type EmbeddedPlugin = (configuration: any) => RumPlugin
+
 export interface RumPublicApiOptions {
   ignoreInitIfSyntheticsWillInjectRum?: boolean
   startDeflateWorker?: (
@@ -631,6 +634,7 @@ export interface RumPublicApiOptions {
   ) => DeflateWorker | undefined
   createDeflateEncoder?: (worker: DeflateWorker, streamId: DeflateEncoderStreamId) => DeflateEncoder
   sdkName?: SdkName
+  embeddedPlugins?: Record<string, EmbeddedPlugin>
 }
 
 export interface Strategy {
@@ -714,7 +718,7 @@ export function makeRumPublicApi(
 
       strategy = createPostStartStrategy(strategy, startRumResult)
 
-      callPluginsMethod(configuration.plugins, 'onRumStart', {
+      callPluginsOnRumStart(configuration.plugins, {
         addEvent: startRumResult.addEvent,
         addError: startRumResult.addError,
       })
@@ -1032,6 +1036,8 @@ export function makeRumPublicApi(
     succeedOperation,
     failOperation,
 
+    ...options.embeddedPlugins,
+
     // Deprecated aliases — kept for backwards compatibility, forward to the renamed APIs above.
     // TODO: remove in the next major version (RUM-16921).
     startFeatureOperation: startOperation,