Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ [RUMF-1353] Add Error.cause property #1740

Merged
merged 28 commits into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
10d8251
[RUMF-1353] Add Error.cause property
Sep 16, 2022
99d7246
remove ts-ignore comments in favour of as TS chaining
Sep 16, 2022
f0e2da8
use ? instead of undefined in type def
Sep 16, 2022
2a6bcfb
mimic formatUnknownError logic in flattenErrorCauses regarding cause …
Sep 20, 2022
1be956a
fix missing export
Sep 21, 2022
5262202
Merge branch 'main' into william/error-cause-property
liywjl Sep 21, 2022
c659327
stack should use type string
Sep 21, 2022
b2f1340
Update packages/core/src/tools/error.spec.ts
liywjl Sep 21, 2022
20776e7
deprecate formatUnknownError in favour of computeRawError
Sep 21, 2022
10a72fc
revert rum-events-format update
Sep 21, 2022
6642d84
add test for flattenErrorCauses with stack
Sep 21, 2022
6a868a1
pass handling as prop
Sep 21, 2022
0f13557
use DEFAULT_RAW_ERROR_PARMS in error spec
Sep 22, 2022
d723a80
causes can be undefinded in errorCollection spec
Sep 22, 2022
6931d26
use originalError name and simplify flattenErrorCauses
Sep 22, 2022
7e7c4c5
update test name
Sep 22, 2022
d66c278
simplify test logic
Sep 22, 2022
4e500b7
Update packages/core/src/tools/error.ts
liywjl Sep 22, 2022
bf6bcdd
fix unit test
Sep 22, 2022
a332178
add test to check stacktrace comes from cause Error
Sep 22, 2022
1e55af3
update test naming
Sep 22, 2022
6d72cf5
update test name and object casting
Sep 22, 2022
de566e7
define error inline in test
Sep 22, 2022
0603c01
merge tests to reduce number of tests
Sep 23, 2022
9c6fd18
🐛 fix wrong stack trace being used in cause errors
Sep 23, 2022
ee8585a
check the cause type is the Error type name
Sep 23, 2022
3536ec1
Merge branch 'main' into william/error-cause-property
liywjl Sep 23, 2022
b8c98fc
Merge branch 'main' into william/error-cause-property
liywjl Oct 3, 2022
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
9 changes: 8 additions & 1 deletion packages/core/src/domain/error/trackRuntimeError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import { startUnhandledErrorCollection } from '../tracekit'

export function trackRuntimeError(errorObservable: Observable<RawError>) {
return startUnhandledErrorCollection((stackTrace, errorObject) => {
const { stack, message, type } = formatUnknownError(stackTrace, errorObject, 'Uncaught')
const { stack, message, type, causes } = formatUnknownError({
stackTrace,
errorObject,
nonErrorPrefix: 'Uncaught',
source: ErrorSource.SOURCE,
})

errorObservable.notify({
message,
stack,
Expand All @@ -15,6 +21,7 @@ export function trackRuntimeError(errorObservable: Observable<RawError>) {
startClocks: clocksNow(),
originalError: errorObject,
handling: ErrorHandling.UNHANDLED,
causes,
})
})
}
3 changes: 3 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ export {
formatUnknownError,
createHandlingStack,
RawError,
RawErrorCause,
ErrorWithCause,
toStackTraceString,
getFileFromStackTraceString,
flattenErrorCauses,
} from './tools/error'
export { Context, ContextArray, ContextValue } from './tools/context'
export { areCookiesAuthorized, getCookie, setCookie, deleteCookie, COOKIE_ACCESS_DELAY } from './browser/cookie'
Expand Down
75 changes: 70 additions & 5 deletions packages/core/src/tools/error.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import type { StackTrace } from '../domain/tracekit'
import { createHandlingStack, formatUnknownError, getFileFromStackTraceString } from './error'
import type { RawErrorCause, ErrorWithCause } from './error'
import {
createHandlingStack,
formatUnknownError,
getFileFromStackTraceString,
flattenErrorCauses,
ErrorSource,
} from './error'

describe('formatUnknownError', () => {
const NOT_COMPUTED_STACK_TRACE: StackTrace = { name: undefined, message: undefined, stack: [] } as any
Expand Down Expand Up @@ -33,7 +40,12 @@ describe('formatUnknownError', () => {
],
}

const formatted = formatUnknownError(stack, undefined, 'Uncaught')
const formatted = formatUnknownError({
stackTrace: stack,
errorObject: undefined,
nonErrorPrefix: 'Uncaught',
source: 'custom',
})

expect(formatted.message).toEqual('oh snap!')
expect(formatted.type).toEqual('TypeError')
Expand All @@ -50,26 +62,64 @@ describe('formatUnknownError', () => {
stack: [],
}

const formatted = formatUnknownError(stack, undefined, 'Uncaught')
const formatted = formatUnknownError({
stackTrace: stack,
liywjl marked this conversation as resolved.
Show resolved Hide resolved
errorObject: undefined,
nonErrorPrefix: 'Uncaught',
source: 'custom',
liywjl marked this conversation as resolved.
Show resolved Hide resolved
})

expect(formatted.message).toEqual('Empty message')
})

it('should format a string error', () => {
const errorObject = 'oh snap!'

const formatted = formatUnknownError(NOT_COMPUTED_STACK_TRACE, errorObject, 'Uncaught')
const formatted = formatUnknownError({
stackTrace: NOT_COMPUTED_STACK_TRACE,
errorObject,
nonErrorPrefix: 'Uncaught',
source: 'custom',
})

expect(formatted.message).toEqual('Uncaught "oh snap!"')
})

it('should format an object error', () => {
const errorObject = { foo: 'bar' }

const formatted = formatUnknownError(NOT_COMPUTED_STACK_TRACE, errorObject, 'Uncaught')
const formatted = formatUnknownError({
stackTrace: NOT_COMPUTED_STACK_TRACE,
errorObject,
nonErrorPrefix: 'Uncaught',
source: 'custom',
})

expect(formatted.message).toEqual('Uncaught {"foo":"bar"}')
})

it('should format an object error with cause', () => {
liywjl marked this conversation as resolved.
Show resolved Hide resolved
const errorObject = new Error('foo: bar') as ErrorWithCause
const nestedErrorObject = new Error('biz: buz') as ErrorWithCause
const deepNestedErrorObject = new Error('fiz: buz') as ErrorWithCause

errorObject.cause = nestedErrorObject
nestedErrorObject.cause = deepNestedErrorObject

const formatted = formatUnknownError({
stackTrace: NOT_COMPUTED_STACK_TRACE,
errorObject,
nonErrorPrefix: 'Uncaught',
source: ErrorSource.SOURCE,
})

expect(formatted.causes?.length).toBe(2)
const causes = formatted.causes as RawErrorCause[]
expect(causes[0].message).toContain(nestedErrorObject.message)
expect(causes[0].source).toContain(ErrorSource.SOURCE)
expect(causes[1].message).toContain(deepNestedErrorObject.message)
expect(causes[1].source).toContain(ErrorSource.SOURCE)
})
})

describe('getFileFromStackTraceString', () => {
Expand Down Expand Up @@ -107,3 +157,18 @@ describe('createHandlingStack', () => {
at userCallOne @ (.*)`)
})
})

describe('flattenErrorCauses', () => {
it('should return empty array if no cause found', () => {
liywjl marked this conversation as resolved.
Show resolved Hide resolved
const error = new Error('foo') as ErrorWithCause
const errorCauses = flattenErrorCauses({ errorObject: error, parentSource: ErrorSource.LOGGER })
expect(errorCauses.length).toEqual(0)
})

it('should only return the first 10 errors if nested chain is longer', () => {
const error = new Error('foo') as ErrorWithCause
error.cause = error
Comment on lines +238 to +240
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one!

const errorCauses = flattenErrorCauses({ errorObject: error, parentSource: ErrorSource.LOGGER })
expect(errorCauses.length).toEqual(10)
})
})
59 changes: 54 additions & 5 deletions packages/core/src/tools/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ import { callMonitored } from './monitor'
import type { ClocksState } from './timeUtils'
import { jsonStringify, noop } from './utils'

export interface ErrorWithCause extends Error {
cause?: Error
}

export type RawErrorCause = {
message: string
source: string
type?: string
stack?: StackTrace
}

export interface RawError {
startClocks: ClocksState
message: string
Expand All @@ -13,6 +24,15 @@ export interface RawError {
originalError?: unknown
handling?: ErrorHandling
handlingStack?: string
causes?: RawErrorCause[]
}

export interface UnknownError {
message: string
stack?: string
handlingStack?: string
type?: string
causes?: RawErrorCause[]
liywjl marked this conversation as resolved.
Show resolved Hide resolved
}

export const ErrorSource = {
Expand All @@ -32,12 +52,21 @@ export const enum ErrorHandling {

export type ErrorSource = typeof ErrorSource[keyof typeof ErrorSource]

export function formatUnknownError(
stackTrace: StackTrace | undefined,
errorObject: any,
nonErrorPrefix: string,
type UnknownErrorParams = {
stackTrace?: StackTrace
errorObject: any
nonErrorPrefix: string
handlingStack?: string
) {
source: ErrorSource
}

export function formatUnknownError({
stackTrace,
errorObject,
nonErrorPrefix,
handlingStack,
source,
}: UnknownErrorParams): UnknownError {
liywjl marked this conversation as resolved.
Show resolved Hide resolved
if (!stackTrace || (stackTrace.message === undefined && !(errorObject instanceof Error))) {
return {
message: `${nonErrorPrefix} ${jsonStringify(errorObject)!}`,
Expand All @@ -47,11 +76,13 @@ export function formatUnknownError(
}
}

const causes = flattenErrorCauses({ errorObject, parentSource: source })
return {
message: stackTrace.message || 'Empty message',
stack: toStackTraceString(stackTrace),
handlingStack,
type: stackTrace.name,
causes,
}
}

Expand Down Expand Up @@ -110,3 +141,21 @@ export function createHandlingStack(): string {

return formattedStack!
}

type FormatErrorCausesParams = { errorObject: ErrorWithCause; parentSource: ErrorSource }

export function flattenErrorCauses({ errorObject, parentSource }: FormatErrorCausesParams): RawErrorCause[] {
liywjl marked this conversation as resolved.
Show resolved Hide resolved
let currentError = errorObject
const causes: RawErrorCause[] = []
while (currentError?.cause && currentError.cause && causes.length < 10) {
liywjl marked this conversation as resolved.
Show resolved Hide resolved
const stackTrace = currentError.cause instanceof Error ? computeStackTrace(currentError.cause) : undefined
causes.push({
message: currentError.cause.message,
source: parentSource,
type: stackTrace?.name,
stack: stackTrace,
})
currentError = currentError.cause
}
return causes
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { RelativeTime, TimeStamp } from '@datadog/browser-core'
import type { RelativeTime, TimeStamp, ErrorWithCause } from '@datadog/browser-core'
import { ErrorHandling, ErrorSource } from '@datadog/browser-core'
import type { TestSetupBuilder } from '../../../../test/specHelper'
import { setup } from '../../../../test/specHelper'
import type { RawRumErrorEvent } from '../../../rawRumEvent.types'
import { RumEventType } from '../../../rawRumEvent.types'
import { LifeCycleEventType } from '../../lifeCycle'
import { doStartErrorCollection } from './errorCollection'
Expand Down Expand Up @@ -49,6 +50,7 @@ describe('error collection', () => {
type: 'Error',
handling: ErrorHandling.HANDLED,
source_type: 'browser',
causes: [],
},
type: RumEventType.ERROR,
view: {
Expand All @@ -61,6 +63,31 @@ describe('error collection', () => {
})
})

it('should extract causes from error', () => {
const { rawRumEvents } = setupBuilder.build()
const error1 = new Error('foo') as ErrorWithCause
const error2 = new Error('bar') as ErrorWithCause
const error3 = new Error('biz') as ErrorWithCause

error1.cause = error2
error2.cause = error3

addError({
error: error1,
handlingStack: 'Error: handling foo',
startClocks: { relative: 1234 as RelativeTime, timeStamp: 123456789 as TimeStamp },
})
const { error } = rawRumEvents[0].rawRumEvent as RawRumErrorEvent
expect(error.message).toEqual('foo')
expect(error.source).toEqual('custom')

expect(error.causes.length).toEqual(2)
expect(error.causes[0].message).toEqual('bar')
expect(error.causes[0].source).toEqual(ErrorSource.LOGGER)
expect(error.causes[1].message).toEqual('biz')
expect(error.causes[1].source).toEqual(ErrorSource.LOGGER)
})

it('should save the specified customer context', () => {
const { rawRumEvents } = setupBuilder.build()
addError({
Expand Down Expand Up @@ -144,6 +171,7 @@ describe('error collection', () => {
type: 'foo',
handling: undefined,
source_type: 'browser',
causes: [],
},
view: {
in_foreground: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ function computeRawError(error: unknown, handlingStack: string, startClocks: Clo
originalError: error,
handling: ErrorHandling.HANDLED,
},
formatUnknownError(stackTrace, error, 'Provided', handlingStack)
formatUnknownError({
stackTrace,
errorObject: error,
nonErrorPrefix: 'Provided',
handlingStack,
source: ErrorSource.CUSTOM,
})
)
}

Expand All @@ -92,6 +98,7 @@ function processError(
handling_stack: error.handlingStack,
type: error.type,
handling: error.handling,
causes: error.causes ?? [],
liywjl marked this conversation as resolved.
Show resolved Hide resolved
source_type: 'browser',
},
type: RumEventType.ERROR as const,
Expand Down
4 changes: 3 additions & 1 deletion packages/rum-core/src/rawRumEvent.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
ResourceType,
ServerDuration,
TimeStamp,
RawErrorCause,
} from '@datadog/browser-core'
import type { RumSessionPlan } from './domain/rumSessionManager'

Expand Down Expand Up @@ -47,7 +48,7 @@ export interface PerformanceResourceDetailsElement {
start: ServerDuration
}

export interface RawRumErrorEvent {
liywjl marked this conversation as resolved.
Show resolved Hide resolved
export type RawRumErrorEvent = {
date: TimeStamp
type: RumEventType.ERROR
error: {
Expand All @@ -58,6 +59,7 @@ export interface RawRumErrorEvent {
source: ErrorSource
message: string
handling?: ErrorHandling
causes: RawErrorCause[]
liywjl marked this conversation as resolved.
Show resolved Hide resolved
source_type: 'browser'
}
view?: {
Expand Down
1 change: 1 addition & 0 deletions packages/rum-core/test/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export function createRawRumEvent(type: RumEventType, overrides?: Context): RawR
source: ErrorSource.SOURCE,
handling: ErrorHandling.HANDLED,
source_type: 'browser',
causes: [],
},
},
overrides
Expand Down