diff --git a/packages/core/src/domain/error/error.spec.ts b/packages/core/src/domain/error/error.spec.ts index 2709b791c8..287a9fbb2f 100644 --- a/packages/core/src/domain/error/error.spec.ts +++ b/packages/core/src/domain/error/error.spec.ts @@ -5,6 +5,30 @@ import type { RawErrorCause, ErrorWithCause } from './error.types' import { ErrorHandling, ErrorSource, NonErrorPrefix } from './error.types' describe('computeRawError', () => { + const ERROR_INSTANCE = new TypeError('oh snap!') + const DEFAULT_STACK = [ + { + args: ['1', 'bar'], + column: 15, + func: 'foo', + line: 52, + url: 'http://path/to/file.js', + }, + { + args: [], + column: undefined, + func: '?', + line: 12, + url: 'http://path/to/file.js', + }, + { + args: ['baz'], + column: undefined, + func: '?', + line: undefined, + url: 'http://path/to/file.js', + }, + ] const NOT_COMPUTED_STACK_TRACE: StackTrace = { name: undefined, message: undefined, stack: [] } as any const DEFAULT_RAW_ERROR_PARAMS = { startClocks: clocksNow(), @@ -12,91 +36,120 @@ describe('computeRawError', () => { source: ErrorSource.CUSTOM, } - it('should format an error', () => { - const stackTrace: StackTrace = { - message: 'oh snap!', - name: 'TypeError', - stack: [ - { - args: ['1', 'bar'], - column: 15, - func: 'foo', - line: 52, - url: 'http://path/to/file.js', - }, - { - args: [], - column: undefined, - func: '?', - line: 12, - url: 'http://path/to/file.js', - }, - { - args: ['baz'], - column: undefined, - func: '?', - line: undefined, - url: 'http://path/to/file.js', - }, - ], - } - - const formatted = computeRawError({ - ...DEFAULT_RAW_ERROR_PARAMS, - stackTrace, - originalError: undefined, - handling: ErrorHandling.HANDLED, - }) - - expect(formatted.message).toEqual('oh snap!') - expect(formatted.type).toEqual('TypeError') - expect(formatted.stack).toEqual(`TypeError: oh snap! + describe('when stackTrace', () => { + describe('from an error instance', () => { + it('should format', () => { + const stackTrace: StackTrace = { + message: 'oh snap!', + name: 'TypeError', + stack: DEFAULT_STACK, + } + + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + stackTrace, + originalError: ERROR_INSTANCE, + handling: ErrorHandling.HANDLED, + }) + + expect(formatted.message).toEqual('oh snap!') + expect(formatted.type).toEqual('TypeError') + expect(formatted.stack).toEqual(`TypeError: oh snap! at foo(1, bar) @ http://path/to/file.js:52:15 at @ http://path/to/file.js:12 at (baz) @ http://path/to/file.js`) - }) - - it('should format an error with an empty message', () => { - const stackTrace: StackTrace = { - message: '', - name: 'TypeError', - stack: [], - } + }) + + it('should format with an empty message', () => { + const stackTrace: StackTrace = { + message: '', + name: 'TypeError', + stack: DEFAULT_STACK, + } + + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + stackTrace, + originalError: ERROR_INSTANCE, + handling: ErrorHandling.HANDLED, + }) + + expect(formatted.message).toEqual('Empty message') + }) + }) - const formatted = computeRawError({ - ...DEFAULT_RAW_ERROR_PARAMS, - stackTrace, - originalError: undefined, - handling: ErrorHandling.HANDLED, + describe('from a string', () => { + it('should format with stack message', () => { + const error = 'Uncaught ReferenceError: foo is undefined' + const stackTrace = { + name: 'ReferenceError', + message: 'foo is undefined', + stack: [{ url: undefined, line: undefined, column: undefined }], + } + + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + stackTrace, + originalError: error, + handling: ErrorHandling.HANDLED, + }) + + expect(formatted.type).toEqual('ReferenceError') + expect(formatted.message).toEqual('foo is undefined') + }) + + it('should format without stack message', () => { + const error = 'oh snap!' + + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + stackTrace: NOT_COMPUTED_STACK_TRACE, + originalError: error, + handling: ErrorHandling.HANDLED, + }) + + expect(formatted.message).toEqual('Uncaught "oh snap!"') + }) }) - expect(formatted.message).toEqual('Empty message') - }) + it('should format an object error', () => { + const error = { foo: 'bar' } - it('should format a string error', () => { - const error = 'oh snap!' + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + stackTrace: NOT_COMPUTED_STACK_TRACE, + originalError: error, + handling: ErrorHandling.HANDLED, + }) - const formatted = computeRawError({ - ...DEFAULT_RAW_ERROR_PARAMS, - stackTrace: NOT_COMPUTED_STACK_TRACE, - originalError: error, - handling: ErrorHandling.HANDLED, + expect(formatted.message).toEqual('Uncaught {"foo":"bar"}') }) - - expect(formatted.message).toEqual('Uncaught "oh snap!"') }) - it('should format an object error', () => { - const error = { foo: 'bar' } + describe('when no stackTrace', () => { + it('should format a string', () => { + const error = 'foo is undefined' - const formatted = computeRawError({ - ...DEFAULT_RAW_ERROR_PARAMS, - stackTrace: NOT_COMPUTED_STACK_TRACE, - originalError: error, - handling: ErrorHandling.HANDLED, + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + originalError: error, + handling: ErrorHandling.HANDLED, + }) + + expect(formatted.message).toEqual('Uncaught "foo is undefined"') }) - expect(formatted.message).toEqual('Uncaught {"foo":"bar"}') + it('should format an object', () => { + const error = { foo: 'bar' } + + const formatted = computeRawError({ + ...DEFAULT_RAW_ERROR_PARAMS, + originalError: error, + handling: ErrorHandling.HANDLED, + }) + + expect(formatted.message).toEqual('Uncaught {"foo":"bar"}') + }) }) it('should set handling according to given parameter', () => { @@ -125,7 +178,13 @@ describe('computeRawError', () => { const stackTrace: StackTrace = { message: 'some typeError message', name: 'TypeError', - stack: [], + stack: [ + { + url: '', + line: 1, + column: 2, + }, + ], } const error = new Error('foo: bar') as ErrorWithCause diff --git a/packages/core/src/domain/error/error.ts b/packages/core/src/domain/error/error.ts index 105d36a9bd..709618ae5c 100644 --- a/packages/core/src/domain/error/error.ts +++ b/packages/core/src/domain/error/error.ts @@ -29,30 +29,40 @@ export function computeRawError({ source, handling, }: RawErrorParams): RawError { - if (!stackTrace || (stackTrace.message === undefined && !(originalError instanceof Error))) { - return { - startClocks, - source, - handling, - originalError, - message: `${nonErrorPrefix} ${jsonStringify(sanitize(originalError))!}`, - stack: NO_ERROR_STACK_PRESENT_MESSAGE, - handlingStack, - type: stackTrace && stackTrace.name, - } - } + const isErrorInstance = originalError instanceof Error + + const message = stackTrace?.message + ? stackTrace.message + : !isErrorInstance + ? `${nonErrorPrefix} ${jsonStringify(sanitize(originalError))!}` + : 'Empty message' + const stack = hasUsableStack(isErrorInstance, stackTrace) + ? toStackTraceString(stackTrace) + : NO_ERROR_STACK_PRESENT_MESSAGE + const causes = isErrorInstance ? flattenErrorCauses(originalError as ErrorWithCause, source) : undefined + const type = stackTrace?.name return { startClocks, source, handling, - originalError, - message: stackTrace.message || 'Empty message', - stack: toStackTraceString(stackTrace), handlingStack, - type: stackTrace.name, - causes: flattenErrorCauses(originalError as ErrorWithCause, source), + originalError, + type, + message, + stack, + causes, + } +} + +function hasUsableStack(isErrorInstance: boolean, stackTrace?: StackTrace): stackTrace is StackTrace { + if (stackTrace === undefined) { + return false + } + if (isErrorInstance) { + return true } + return stackTrace.stack.length > 0 && (stackTrace.stack.length > 1 || stackTrace.stack[0].url !== undefined) } export function toStackTraceString(stack: StackTrace) { diff --git a/packages/core/src/domain/error/trackRuntimeError.spec.ts b/packages/core/src/domain/error/trackRuntimeError.spec.ts index 14ef4ad8e5..454f65d6d1 100644 --- a/packages/core/src/domain/error/trackRuntimeError.spec.ts +++ b/packages/core/src/domain/error/trackRuntimeError.spec.ts @@ -1,9 +1,10 @@ import { Observable } from '../../tools/observable' +import { collectAsyncCalls } from '../../../test' import { NO_ERROR_STACK_PRESENT_MESSAGE } from './error' import { trackRuntimeError } from './trackRuntimeError' import type { RawError } from './error.types' -describe('runtime error tracker', () => { +describe('trackRuntimeError', () => { const ERROR_MESSAGE = 'foo' let originalOnErrorHandler: OnErrorEventHandler @@ -39,12 +40,11 @@ describe('runtime error tracker', () => { it('should call original error handler', (done) => { setTimeout(() => { throw new Error(ERROR_MESSAGE) - }, 10) - - setTimeout(() => { + }) + collectAsyncCalls(onErrorSpy, 1, () => { expect(onErrorSpy.calls.mostRecent().args[0]).toMatch(ERROR_MESSAGE) done() - }, 100) + }) }) it('should call original unhandled rejection handler', () => { @@ -55,27 +55,53 @@ describe('runtime error tracker', () => { expect(onUnhandledrejectionSpy.calls.mostRecent().args[0].reason).toMatch(ERROR_MESSAGE) }) - it('should notify error', (done) => { + it('should notify unhandled error instance', (done) => { setTimeout(() => { throw new Error(ERROR_MESSAGE) - }, 10) + }) + collectAsyncCalls(onErrorSpy, 1, () => { + const collectedError = notifyError.calls.mostRecent().args[0] as RawError + expect(collectedError.message).toEqual(ERROR_MESSAGE) + expect(collectedError.stack).not.toEqual(NO_ERROR_STACK_PRESENT_MESSAGE) + done() + }) + }) + it('should notify unhandled string', (done) => { setTimeout(() => { - expect((notifyError.calls.mostRecent().args[0] as RawError).message).toEqual(ERROR_MESSAGE) + // eslint-disable-next-line no-throw-literal + throw 'foo' + }) + collectAsyncCalls(onErrorSpy, 1, () => { + const collectedError = notifyError.calls.mostRecent().args[0] as RawError + expect(collectedError.message).toEqual('Uncaught "foo"') + expect(collectedError.stack).toEqual(NO_ERROR_STACK_PRESENT_MESSAGE) done() - }, 100) + }) }) - it('should handle direct onerror calls with objects', (done) => { + it('should notify unhandled object', (done) => { setTimeout(() => { - window.onerror!({ foo: 'bar' } as any) - }, 10) + // eslint-disable-next-line no-throw-literal + throw { a: 'foo' } + }) + collectAsyncCalls(onErrorSpy, 1, () => { + const collectedError = notifyError.calls.mostRecent().args[0] as RawError + expect(collectedError.message).toEqual('Uncaught {"a":"foo"}') + expect(collectedError.stack).toEqual(NO_ERROR_STACK_PRESENT_MESSAGE) + done() + }) + }) + it('should handle direct onerror calls with objects', (done) => { setTimeout(() => { + window.onerror!({ foo: 'bar' } as any) + }) + collectAsyncCalls(onErrorSpy, 1, () => { const collectedError = notifyError.calls.mostRecent().args[0] as RawError expect(collectedError.message).toEqual('Uncaught {"foo":"bar"}') expect(collectedError.stack).toEqual(NO_ERROR_STACK_PRESENT_MESSAGE) done() - }, 100) + }) }) }) diff --git a/packages/core/src/domain/tracekit/tracekit.ts b/packages/core/src/domain/tracekit/tracekit.ts index a0f3e5f513..caf075b054 100644 --- a/packages/core/src/domain/tracekit/tracekit.ts +++ b/packages/core/src/domain/tracekit/tracekit.ts @@ -1,6 +1,6 @@ import { instrumentMethodAndCallOriginal } from '../../tools/instrumentMethod' import { computeStackTrace } from './computeStackTrace' -import type { UnhandledErrorCallback, StackTrace } from './types' +import type { UnhandledErrorCallback } from './types' // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Error_types const ERROR_TYPES_RE = @@ -55,41 +55,32 @@ export function startUnhandledErrorCollection(callback: UnhandledErrorCallback) */ function instrumentOnError(callback: UnhandledErrorCallback) { return instrumentMethodAndCallOriginal(window, 'onerror', { - before(this: any, message: Event | string, url?: string, lineNo?: number, columnNo?: number, errorObj?: Error) { - let stack: StackTrace - + before(this: any, messageObj: unknown, url?: string, line?: number, column?: number, errorObj?: unknown) { if (errorObj) { - stack = computeStackTrace(errorObj) - callback(stack, errorObj) - } else { - const location = { - url, - column: columnNo, - line: lineNo, - } - - let name - let msg = message - if ({}.toString.call(message) === '[object String]') { - const groups = ERROR_TYPES_RE.exec(msg as string) - if (groups) { - name = groups[1] - msg = groups[2] - } - } - - stack = { - name, - message: typeof msg === 'string' ? msg : undefined, - stack: [location], - } - - callback(stack, message) + callback(computeStackTrace(errorObj), errorObj) + return } + const stack = [{ url, column, line }] + const { name, message } = tryToParseMessage(messageObj) + const stackTrace = { + name, + message, + stack, + } + callback(stackTrace, messageObj) }, }) } +function tryToParseMessage(messageObj: unknown) { + let name + let message + if ({}.toString.call(messageObj) === '[object String]') { + ;[, name, message] = ERROR_TYPES_RE.exec(messageObj as string)! + } + return { name, message } +} + /** * Install a global onunhandledrejection handler */ diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts index 3b3770a2fd..7ab6b4c8ea 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts @@ -1,5 +1,5 @@ import type { RelativeTime, TimeStamp, ErrorWithCause } from '@datadog/browser-core' -import { ErrorHandling, ErrorSource } from '@datadog/browser-core' +import { ErrorHandling, ErrorSource, NO_ERROR_STACK_PRESENT_MESSAGE } from '@datadog/browser-core' import type { TestSetupBuilder } from '../../../../test' import { setup } from '../../../../test' import type { RawRumErrorEvent } from '../../../rawRumEvent.types' @@ -32,41 +32,64 @@ describe('error collection', () => { setupBuilder.cleanup() }) - describe('provided', () => { - it('notifies a raw rum error event', () => { - const { rawRumEvents } = setupBuilder.build() - const error = new Error('foo') - - addError({ - error, - handlingStack: 'Error: handling foo', - startClocks: { relative: 1234 as RelativeTime, timeStamp: 123456789 as TimeStamp }, - }) - - expect(rawRumEvents.length).toBe(1) - expect(rawRumEvents[0]).toEqual({ - customerContext: undefined, - rawRumEvent: { - date: jasmine.any(Number), - error: { - id: jasmine.any(String), - message: 'foo', - source: ErrorSource.CUSTOM, - stack: jasmine.stringMatching('Error: foo'), - handling_stack: 'Error: handling foo', - type: 'Error', - handling: ErrorHandling.HANDLED, - source_type: 'browser', - causes: undefined, - }, - type: RumEventType.ERROR, - view: { - in_foreground: true, + describe('addError', () => { + ;[ + { + testCase: 'an error instance', + error: new Error('foo'), + message: 'foo', + type: 'Error', + stack: jasmine.stringMatching('Error: foo'), + }, + { + testCase: 'a string', + error: 'foo', + message: 'Provided "foo"', + type: undefined, + stack: NO_ERROR_STACK_PRESENT_MESSAGE, + }, + { + testCase: 'an object', + error: { a: 'foo' }, + message: 'Provided {"a":"foo"}', + type: undefined, + stack: NO_ERROR_STACK_PRESENT_MESSAGE, + }, + ].forEach(({ testCase, error, message, type, stack }) => { + it(`notifies a raw rum error event from ${testCase}`, () => { + const { rawRumEvents } = setupBuilder.build() + + addError({ + error, + handlingStack: 'Error: handling foo', + startClocks: { relative: 1234 as RelativeTime, timeStamp: 123456789 as TimeStamp }, + }) + + expect(rawRumEvents.length).toBe(1) + expect(rawRumEvents[0]).toEqual({ + customerContext: undefined, + rawRumEvent: { + date: jasmine.any(Number), + error: { + id: jasmine.any(String), + message, + source: ErrorSource.CUSTOM, + stack, + handling_stack: 'Error: handling foo', + type, + handling: ErrorHandling.HANDLED, + source_type: 'browser', + causes: undefined, + }, + type: RumEventType.ERROR, + view: { + in_foreground: true, + }, }, - }, - savedCommonContext: undefined, - startTime: 1234 as RelativeTime, - domainContext: { error }, + savedCommonContext: undefined, + startTime: 1234 as RelativeTime, + domainContext: { error }, + }) }) })