Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/browser-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
42 changes: 42 additions & 0 deletions packages/browser-core/src/tools/thenable.spec.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
23 changes: 23 additions & 0 deletions packages/browser-core/src/tools/thenable.ts
Comment thread
BeltranBulbarellaDD marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -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))
}
39 changes: 39 additions & 0 deletions packages/browser-rum-core/src/boot/preStartRum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
})

Expand Down
59 changes: 38 additions & 21 deletions packages/browser-rum-core/src/boot/preStartRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Comment thread
BenoitZugmeyer marked this conversation as resolved.
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 = callPluginsMethod(initConfiguration.plugins, 'onInit', { initConfiguration, publicApi })

if (typeof shouldContinue === 'boolean') {
if (shouldContinue) {
proceedWithInit()
}
} else {
doInit(initConfiguration, errorStack)
shouldContinue
.then((result) => {
if (result) {
proceedWithInit()
}
})
.catch(monitorError)
}
},

Expand Down
72 changes: 70 additions & 2 deletions packages/browser-rum-core/src/domain/plugins.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('callPluginsMethod', () => {
const plugin1 = { name: 'a', onInit: jasmine.createSpy() } satisfies RumPlugin
const plugin2 = { name: 'b', onInit: jasmine.createSpy() } satisfies RumPlugin
const parameter = { initConfiguration: {} as RumInitConfiguration, publicApi: {} as RumPublicApi }
callPluginsMethod([plugin1, plugin2], 'onInit', parameter)
void callPluginsMethod([plugin1, plugin2], 'onInit', parameter)
expect(plugin1.onInit).toHaveBeenCalledWith(parameter)
expect(plugin2.onInit).toHaveBeenCalledWith(parameter)
})
Expand All @@ -17,7 +17,75 @@ describe('callPluginsMethod', () => {
const plugin1 = { name: 'a', onInit: jasmine.createSpy() } satisfies RumPlugin
const plugin2 = { name: 'b' } satisfies RumPlugin
const parameter = { initConfiguration: {} as RumInitConfiguration, publicApi: {} as RumPublicApi }
callPluginsMethod([plugin1, plugin2], 'onInit', parameter)
void callPluginsMethod([plugin1, plugin2], 'onInit', parameter)
expect(plugin1.onInit).toHaveBeenCalledWith(parameter)
})
})

describe('works with sync and async plugins onInit', () => {
const PARAMETER = { initConfiguration: {} as RumInitConfiguration, publicApi: {} as RumPublicApi }

it('returns true synchronously when there are no plugins', () => {
expect(callPluginsMethod(undefined, 'onInit', PARAMETER)).toBe(true)
})

it('returns true synchronously when every plugin returns void or true', () => {
const plugin1 = { name: 'a', onInit: jasmine.createSpy().and.returnValue(undefined) } satisfies RumPlugin
const plugin2 = { name: 'b', onInit: jasmine.createSpy().and.returnValue(true) } satisfies RumPlugin

const result = callPluginsMethod([plugin1, plugin2], 'onInit', PARAMETER)

expect(result).toBe(true)
expect(plugin1.onInit).toHaveBeenCalledWith(PARAMETER)
expect(plugin2.onInit).toHaveBeenCalledWith(PARAMETER)
})

it('returns false synchronously as soon as a sync plugin returns false, without calling the rest', () => {
const plugin1 = { name: 'a', onInit: jasmine.createSpy().and.returnValue(false) } satisfies RumPlugin
const plugin2 = { name: 'b', onInit: jasmine.createSpy() } satisfies RumPlugin

const result = callPluginsMethod([plugin1, plugin2], 'onInit', PARAMETER)

expect(result).toBe(false)
expect(plugin2.onInit).not.toHaveBeenCalled()
})

it('returns a Promise once a plugin returns a thenable, and resolves to true if nothing aborts', async () => {
const plugin1 = { name: 'a', onInit: () => Promise.resolve() } satisfies RumPlugin
const plugin2 = { name: 'b', onInit: jasmine.createSpy().and.returnValue(true) } satisfies RumPlugin

const result = callPluginsMethod([plugin1, plugin2], 'onInit', PARAMETER)

expect(result).not.toBe(true)
expect(await result).toBe(true)
expect(plugin2.onInit).toHaveBeenCalledWith(PARAMETER)
})

it('resolves to false and stops calling further plugins once an async plugin resolves to false', async () => {
const plugin1 = { name: 'a', onInit: () => Promise.resolve(false) } satisfies RumPlugin
const plugin2 = { name: 'b', onInit: jasmine.createSpy() } satisfies RumPlugin

const result = callPluginsMethod([plugin1, plugin2], 'onInit', PARAMETER)

expect(await result).toBe(false)
expect(plugin2.onInit).not.toHaveBeenCalled()
})

it('lets a sync plugin see mutations made by an earlier async plugin', async () => {
const plugin1: RumPlugin = {
name: 'a',
onInit: ({ initConfiguration }) => {
initConfiguration.clientToken = 'from-async-plugin'
return Promise.resolve()
},
}
const plugin2: RumPlugin = {
name: 'b',
onInit: ({ initConfiguration }) => {
expect(initConfiguration.clientToken).toBe('from-async-plugin')
},
}

await callPluginsMethod([plugin1, plugin2], 'onInit', PARAMETER)
})
})
69 changes: 63 additions & 6 deletions packages/browser-rum-core/src/domain/plugins.ts

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 suggestion: If we want to decouple the logic for the onInit and onStart hooks, I would create two separate functions. We could keep it relatively simple:

/**
 * Calls each plugin's `onInit`, and returns whether the initialization should go on: `false` if any
 * plugin aborts it. Stays synchronous as long as no plugin returns a thenable.
 */
export function callPluginsOnInit(
  plugins: RumPlugin[] | undefined,
  parameter: { initConfiguration: RumInitConfiguration; publicApi: RumPublicApi }
): boolean | Promise<boolean> {
  const results = (plugins ?? []).map((plugin) => plugin.onInit?.(parameter))

  if (results.some(isThenable)) {
    return Promise.all(results.map((result) => Promise.resolve(result))).then(
      (resolvedResults) => !resolvedResults.includes(false)
    )
  }
  return !results.includes(false)
}

export function callPluginsOnRumStart(plugins: RumPlugin[] | undefined, options: OnRumStartOptions): void {
  for (const plugin of plugins ?? []) {
    plugin.onRumStart?.(options)
  }
}

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isTimeoutError, isThenable, waitForThenable } from '@datadog/browser-core'
import type { RumPublicApi } from '../boot/rumPublicApi'
import type { StartRumResult } from '../boot/startRum'
import type { RumInitConfiguration } from './configuration'
Expand Down Expand Up @@ -25,35 +26,91 @@ export interface OnRumStartOptions {
* notice. Please use only plugins provided by Datadog matching the version of the SDK you are
* using.
*
* `onInit` may abort the SDK initialization by returning (or resolving to) `false`. Returning a
* Promise defers the remaining plugins' `onInit` calls and the actual initialization until it
* resolves.
*
* @experimental
*/
export interface RumPlugin {
name: string
getConfigurationTelemetry?(): Record<string, unknown>
onInit?(options: { initConfiguration: RumInitConfiguration; publicApi: RumPublicApi }): void
onInit?(options: {
initConfiguration: RumInitConfiguration
publicApi: RumPublicApi
}): false | void | Promise<false | void>
onRumStart?(options: OnRumStartOptions): void
}

type MethodNames = 'onInit' | 'onRumStart'
type MethodParameter<MethodName extends MethodNames> = Parameters<NonNullable<RumPlugin[MethodName]>>[0]

export function callPluginsMethod<MethodName extends MethodNames>(
export function callPluginsMethod(
plugins: RumPlugin[] | undefined,
methodName: MethodName,
parameter: MethodParameter<MethodName>
methodName: 'onInit',
parameter: MethodParameter<'onInit'>
): boolean | Promise<boolean>
export function callPluginsMethod(
plugins: RumPlugin[] | undefined,
methodName: 'onRumStart',
parameter: MethodParameter<'onRumStart'>
): void
export function callPluginsMethod<MethodName extends MethodNames>(
plugins: RumPlugin[] | undefined,
methodName: MethodName,
parameter: any
) {
): any {
if (methodName === 'onInit') {
return runOnInitPlugins(plugins, parameter)
}
if (!plugins) {
return
}
for (const plugin of plugins) {
const method = plugin[methodName]
if (method) {
method(parameter)
// nothing apart from onInit is expected to return a value
void method(parameter)
}
}
}

const DEFAULT_ON_INIT_TIMEOUT = 3000

/**
* Calls each plugin's `onInit` method in order, stopping (synchronously or asynchronously) as
* soon as one returns `false`. Stays synchronous as long as no plugin returns a thenable.
*/
function runOnInitPlugins(
plugins: RumPlugin[] | undefined,
parameter: { initConfiguration: RumInitConfiguration; publicApi: RumPublicApi }
): boolean | Promise<boolean> {
if (!plugins) {
return true
}
let index = 0

function next(): boolean | Promise<boolean> {
while (index < plugins!.length) {
const result = plugins![index++].onInit?.(parameter)
if (isThenable<boolean | void>(result)) {
return waitForThenable(result, DEFAULT_ON_INIT_TIMEOUT)
.then((resolved) => (resolved === false ? false : next()))
.catch((reason) => {
if (isTimeoutError(reason)) {
throw new Error(
`Plugin ${plugins![index - 1].name} onInit() timed out after ${DEFAULT_ON_INIT_TIMEOUT}ms`
)
}
throw reason
})
}
if (result === false) {
return false
}
}
return true
}

return next()
}
Loading
Loading