Skip to content

Commit

Permalink
report the original caught error by addReactError
Browse files Browse the repository at this point in the history
  • Loading branch information
eloytoro committed Jan 15, 2025
1 parent 4623045 commit 62cf07d
Show file tree
Hide file tree
Showing 16 changed files with 124 additions and 48 deletions.
3 changes: 3 additions & 0 deletions packages/core/src/domain/error/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type RawErrorParams = {
originalError: unknown

handlingStack?: string
componentStack?: string
startClocks: ClocksState
nonErrorPrefix: NonErrorPrefix
source: ErrorSource
Expand All @@ -23,6 +24,7 @@ export function computeRawError({
stackTrace,
originalError,
handlingStack,
componentStack,
startClocks,
nonErrorPrefix,
source,
Expand All @@ -43,6 +45,7 @@ export function computeRawError({
source,
handling,
handlingStack,
componentStack,
originalError,
type,
message,
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/domain/error/error.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface RawError {
originalError?: unknown
handling?: ErrorHandling
handlingStack?: string
componentStack?: string
causes?: RawErrorCause[]
fingerprint?: string
csp?: Csp
Expand Down
4 changes: 3 additions & 1 deletion packages/rum-core/src/boot/preStartRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export function createPreStartStrategy(
bufferApiCalls.add((startRumResult) => startRumResult.addDurationVital(vital))
}

return {
const strategy: Strategy = {
init(initConfiguration, publicApi) {
if (!initConfiguration) {
display.error('Missing configuration')
Expand Down Expand Up @@ -229,6 +229,8 @@ export function createPreStartStrategy(

addDurationVital,
}

return strategy
}

function overrideInitConfigurationForBridge(initConfiguration: RumInitConfiguration): RumInitConfiguration {
Expand Down
3 changes: 3 additions & 0 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import type { DurationVitalReference } from '../domain/vital/vitalCollection'
import { createCustomVitalsState } from '../domain/vital/vitalCollection'
import { createPreStartStrategy } from './preStartRum'
import type { StartRum, StartRumResult } from './startRum'
import { callPluginsMethod } from '../domain/plugins'

export interface StartRecordingOptions {
force: boolean
Expand Down Expand Up @@ -421,6 +422,8 @@ export function makeRumPublicApi(

strategy = createPostStartStrategy(strategy, startRumResult)

callPluginsMethod(configuration.plugins, 'onRumStart', { strategy })

return startRumResult
}
)
Expand Down
3 changes: 3 additions & 0 deletions packages/rum-core/src/domain/error/errorCollection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ describe('error collection', () => {
source: ErrorSource.CUSTOM,
stack,
handling_stack: 'Error: handling foo',
component_stack: undefined,
type,
handling: ErrorHandling.HANDLED,
source_type: 'browser',
Expand Down Expand Up @@ -273,6 +274,7 @@ describe('error collection', () => {
type: 'foo',
originalError: error,
handlingStack: 'Error: handling foo',
componentStack: 'at div',
},
})

Expand All @@ -285,6 +287,7 @@ describe('error collection', () => {
source: ErrorSource.CUSTOM,
stack: 'bar',
handling_stack: 'Error: handling foo',
component_stack: 'at div',
type: 'foo',
handling: undefined,
source_type: 'browser',
Expand Down
6 changes: 4 additions & 2 deletions packages/rum-core/src/domain/error/errorCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface ProvidedError {
error: unknown
context?: Context
handlingStack: string
componentStack?: string
}

export function startErrorCollection(
Expand All @@ -47,7 +48,6 @@ export function startErrorCollection(

return doStartErrorCollection(lifeCycle, pageStateHistory, featureFlagContexts)
}

export function doStartErrorCollection(
lifeCycle: LifeCycle,
pageStateHistory: PageStateHistory,
Expand All @@ -63,14 +63,15 @@ export function doStartErrorCollection(

return {
addError: (
{ error, handlingStack, startClocks, context: customerContext }: ProvidedError,
{ error, handlingStack, componentStack, startClocks, context: customerContext }: ProvidedError,
savedCommonContext?: CommonContext
) => {
const stackTrace = isError(error) ? computeStackTrace(error) : undefined
const rawError = computeRawError({
stackTrace,
originalError: error,
handlingStack,
componentStack,
startClocks,
nonErrorPrefix: NonErrorPrefix.PROVIDED,
source: ErrorSource.CUSTOM,
Expand Down Expand Up @@ -99,6 +100,7 @@ function processError(
source: error.source,
stack: error.stack,
handling_stack: error.handlingStack,
component_stack: error.componentStack,
type: error.type,
handling: error.handling,
causes: error.causes,
Expand Down
10 changes: 8 additions & 2 deletions packages/rum-core/src/domain/plugins.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
import type { RumPublicApi } from '../boot/rumPublicApi'
import type { RumPublicApi, Strategy } from '../boot/rumPublicApi'
import type { RumInitConfiguration } from './configuration'

export interface RumPlugin {
name: string
getConfigurationTelemetry?(): Record<string, unknown>
onInit?(options: { initConfiguration: RumInitConfiguration; publicApi: RumPublicApi }): void
onRumStart?(options: { strategy: Strategy }): void
}

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

export function callPluginsMethod<MethodName extends MethodNames>(
plugins: RumPlugin[] | undefined,
methodName: MethodName,
parameter: MethodParameter<MethodName>
): void
export function callPluginsMethod<MethodName extends MethodNames>(
plugins: RumPlugin[] | undefined,
methodName: MethodName,
parameter: any
) {
if (!plugins) {
return
Expand Down
2 changes: 1 addition & 1 deletion packages/rum-core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { RumPublicApi, makeRumPublicApi, RecorderApi, StartRecordingOptions } from './boot/rumPublicApi'
export { RumPublicApi, makeRumPublicApi, RecorderApi, StartRecordingOptions, Strategy } from './boot/rumPublicApi'
export { StartRum } from './boot/startRum'
export {
RumEvent,
Expand Down
1 change: 1 addition & 0 deletions packages/rum-core/src/rawRumEvent.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export interface RawRumErrorEvent {
type?: string
stack?: string
handling_stack?: string
component_stack?: string
fingerprint?: string
source: ErrorSource
message: string
Expand Down
23 changes: 15 additions & 8 deletions packages/rum-react/src/domain/error/addReactError.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,26 @@ describe('addReactError', () => {
it('reports the error to the SDK', () => {
const addErrorSpy = jasmine.createSpy()
initializeReactPlugin({
publicApi: {
strategy: {
addError: addErrorSpy,
},
})
const originalError = new Error('error')
const originalError = new Error('error message')
originalError.name = 'CustomError'

addReactError(originalError, { componentStack: 'at ComponentSpy toto.js' })

expect(addErrorSpy).toHaveBeenCalledOnceWith(jasmine.any(Error), { framework: 'react' })
const error = addErrorSpy.calls.first().args[0]
expect(error.message).toBe('error')
expect(error.name).toBe('ReactRenderingError')
expect(error.stack).toContain('at ComponentSpy')
expect(error.cause).toBe(originalError)
expect(addErrorSpy).toHaveBeenCalledOnceWith({
error: jasmine.any(Error),
handlingStack: jasmine.any(String),
componentStack: jasmine.stringContaining('at ComponentSpy'),
context: { framework: 'react' },
startClocks: jasmine.anything(),
})
const { error } = addErrorSpy.calls.first().args[0]
expect(error.message).toBe(originalError.message)
expect(error.name).toBe(originalError.name)
expect(error.stack).toBe(originalError.stack)
expect(error.cause).toBe(undefined)
})
})
21 changes: 13 additions & 8 deletions packages/rum-react/src/domain/error/addReactError.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import type { ErrorInfo } from 'react'
import type { ErrorWithCause } from '@datadog/browser-core'
import { onReactPluginInit } from '../reactPlugin'
import { callMonitored, clocksNow, createHandlingStack } from '@datadog/browser-core'
import { onRumStart } from '../reactPlugin'

export function addReactError(error: Error, info: ErrorInfo) {
const renderingError = new Error(error.message)
renderingError.name = 'ReactRenderingError'
renderingError.stack = info.componentStack ?? undefined
;(renderingError as ErrorWithCause).cause = error
onReactPluginInit((_, rumPublicApi) => {
rumPublicApi.addError(renderingError, { framework: 'react' })
onRumStart((strategy) => {
const handlingStack = createHandlingStack()
callMonitored(() => {
strategy.addError({
error,
handlingStack,
componentStack: info.componentStack ?? undefined,
context: { framework: 'react' },
startClocks: clocksNow(),
})
})
})
}
19 changes: 12 additions & 7 deletions packages/rum-react/src/domain/error/errorBoundary.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('ErrorBoundary', () => {
it('reports the error to the SDK', () => {
const addErrorSpy = jasmine.createSpy()
initializeReactPlugin({
publicApi: {
strategy: {
addError: addErrorSpy,
},
})
Expand All @@ -91,11 +91,16 @@ describe('ErrorBoundary', () => {
</ErrorBoundary>
)

expect(addErrorSpy).toHaveBeenCalledOnceWith(jasmine.any(Error), { framework: 'react' })
const error = addErrorSpy.calls.first().args[0]
expect(error.message).toBe('error')
expect(error.name).toBe('ReactRenderingError')
expect(error.stack).toContain('ComponentSpy')
expect(error.cause).toBe(originalError)
expect(addErrorSpy).toHaveBeenCalledOnceWith({
error: jasmine.any(Error),
handlingStack: jasmine.any(String),
componentStack: jasmine.stringContaining('ComponentSpy'),
context: { framework: 'react' },
startClocks: jasmine.anything(),
})
const { error } = addErrorSpy.calls.first().args[0]
expect(error.message).toBe(originalError.message)
expect(error.name).toBe(originalError.name)
expect(error.cause).toBe(undefined)
})
})
25 changes: 17 additions & 8 deletions packages/rum-react/src/domain/reactPlugin.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { RumInitConfiguration, RumPublicApi } from '@datadog/browser-rum-core'
import { onReactPluginInit, reactPlugin, resetReactPlugin } from './reactPlugin'
import type { RumInitConfiguration, RumPublicApi, Strategy } from '@datadog/browser-rum-core'
import { onRumInit, reactPlugin, resetReactPlugin } from './reactPlugin'

const PUBLIC_API = {} as RumPublicApi
const INIT_CONFIGURATION = {} as RumInitConfiguration
const STRATEGY = {} as Strategy

describe('reactPlugin', () => {
afterEach(() => {
Expand All @@ -22,11 +23,15 @@ describe('reactPlugin', () => {
it('calls callbacks registered with onReactPluginInit during onInit', () => {
const callbackSpy = jasmine.createSpy()
const pluginConfiguration = {}
onReactPluginInit(callbackSpy)
onRumInit(callbackSpy)

expect(callbackSpy).not.toHaveBeenCalled()

reactPlugin(pluginConfiguration).onInit({ publicApi: PUBLIC_API, initConfiguration: INIT_CONFIGURATION })
reactPlugin(pluginConfiguration).onInit({
publicApi: PUBLIC_API,
initConfiguration: INIT_CONFIGURATION,
strategy: STRATEGY,
})

expect(callbackSpy).toHaveBeenCalledTimes(1)
expect(callbackSpy.calls.mostRecent().args[0]).toBe(pluginConfiguration)
Expand All @@ -36,9 +41,13 @@ describe('reactPlugin', () => {
it('calls callbacks immediately if onInit was already invoked', () => {
const callbackSpy = jasmine.createSpy()
const pluginConfiguration = {}
reactPlugin(pluginConfiguration).onInit({ publicApi: PUBLIC_API, initConfiguration: INIT_CONFIGURATION })
reactPlugin(pluginConfiguration).onInit({
publicApi: PUBLIC_API,
initConfiguration: INIT_CONFIGURATION,
strategy: STRATEGY,
})

onReactPluginInit(callbackSpy)
onRumInit(callbackSpy)

expect(callbackSpy).toHaveBeenCalledTimes(1)
expect(callbackSpy.calls.mostRecent().args[0]).toBe(pluginConfiguration)
Expand All @@ -47,14 +56,14 @@ describe('reactPlugin', () => {

it('enforce manual view tracking when router is enabled', () => {
const initConfiguration = { ...INIT_CONFIGURATION }
reactPlugin({ router: true }).onInit({ publicApi: PUBLIC_API, initConfiguration })
reactPlugin({ router: true }).onInit({ publicApi: PUBLIC_API, initConfiguration, strategy: STRATEGY })

expect(initConfiguration.trackViewsManually).toBe(true)
})

it('does not enforce manual view tracking when router is disabled', () => {
const initConfiguration = { ...INIT_CONFIGURATION }
reactPlugin({ router: false }).onInit({ publicApi: PUBLIC_API, initConfiguration })
reactPlugin({ router: false }).onInit({ publicApi: PUBLIC_API, initConfiguration, strategy: STRATEGY })

expect(initConfiguration.trackViewsManually).toBeUndefined()
})
Expand Down
35 changes: 28 additions & 7 deletions packages/rum-react/src/domain/reactPlugin.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type { RumPlugin, RumPublicApi } from '@datadog/browser-rum-core'
import type { RumPlugin, RumPublicApi, Strategy } from '@datadog/browser-rum-core'

let globalPublicApi: RumPublicApi | undefined
let globalConfiguration: ReactPluginConfiguration | undefined
type Subscriber = (configuration: ReactPluginConfiguration, rumPublicApi: RumPublicApi) => void
let globalStrategy: Strategy | undefined
type InitSubscriber = (configuration: ReactPluginConfiguration, rumPublicApi: RumPublicApi) => void
type StartSubscriber = (strategy: Strategy) => void

const onReactPluginInitSubscribers: Subscriber[] = []
const onRumInitSubscribers: InitSubscriber[] = []
const onRumStartSubscribers: StartSubscriber[] = []

export interface ReactPluginConfiguration {
/**
Expand All @@ -19,27 +22,45 @@ export function reactPlugin(configuration: ReactPluginConfiguration = {}) {
onInit({ publicApi, initConfiguration }) {
globalPublicApi = publicApi
globalConfiguration = configuration
onReactPluginInitSubscribers.forEach((subscriber) => subscriber(configuration, publicApi))
for (const subscriber of onRumInitSubscribers) {
subscriber(globalConfiguration, globalPublicApi)
}
if (configuration.router) {
initConfiguration.trackViewsManually = true
}
},
onRumStart({ strategy }) {
globalStrategy = strategy
for (const subscriber of onRumStartSubscribers) {
subscriber(strategy)
}
},
getConfigurationTelemetry() {
return { router: !!configuration.router }
},
} satisfies RumPlugin
}

export function onReactPluginInit(callback: Subscriber) {
export function onRumInit(callback: InitSubscriber) {
if (globalConfiguration && globalPublicApi) {
callback(globalConfiguration, globalPublicApi)
} else {
onReactPluginInitSubscribers.push(callback)
onRumInitSubscribers.push(callback)
}
}

export function onRumStart(callback: StartSubscriber) {
if (globalStrategy) {
callback(globalStrategy)
} else {
onRumStartSubscribers.push(callback)
}
}

export function resetReactPlugin() {
globalPublicApi = undefined
globalConfiguration = undefined
onReactPluginInitSubscribers.length = 0
globalStrategy = undefined
onRumInitSubscribers.length = 0
onRumStartSubscribers.length = 0
}
Loading

0 comments on commit 62cf07d

Please sign in to comment.