From 2847879d9eb391ebb580ed19b6ef1043e4212556 Mon Sep 17 00:00:00 2001 From: David Newell Date: Mon, 14 Oct 2024 12:22:49 +0100 Subject: [PATCH 1/5] chore: improve exception autocapture --- .../exception-autocapture/error-conversion.ts | 55 ++++++--- .../exception-autocapture/stack-trace.ts | 114 ++++++++++++------ 2 files changed, 109 insertions(+), 60 deletions(-) diff --git a/src/extensions/exception-autocapture/error-conversion.ts b/src/extensions/exception-autocapture/error-conversion.ts index 26398a9b5..e5f6e8c82 100644 --- a/src/extensions/exception-autocapture/error-conversion.ts +++ b/src/extensions/exception-autocapture/error-conversion.ts @@ -10,7 +10,7 @@ import { } from './type-checking' import { defaultStackParser, StackFrame } from './stack-trace' -import { isEmptyString, isNumber, isString, isUndefined } from '../../utils/type-utils' +import { isEmptyString, isString, isUndefined } from '../../utils/type-utils' import { ErrorEventArgs, ErrorProperties, SeverityLevel, severityLevels } from '../../types' /** @@ -20,32 +20,16 @@ import { ErrorEventArgs, ErrorProperties, SeverityLevel, severityLevels } from ' const ERROR_TYPES_PATTERN = /^(?:[Uu]ncaught (?:exception: )?)?(?:((?:Eval|Internal|Range|Reference|Syntax|Type|URI|)Error): )?(.*)$/i -const reactMinifiedRegexp = /Minified React error #\d+;/i - -function getPopSize(ex: Error & { framesToPop?: number }): number { - if (ex) { - if (isNumber(ex.framesToPop)) { - return ex.framesToPop - } - - if (reactMinifiedRegexp.test(ex.message)) { - return 1 - } - } - - return 0 -} - export function parseStackFrames(ex: Error & { framesToPop?: number; stacktrace?: string }): StackFrame[] { // Access and store the stacktrace property before doing ANYTHING // else to it because Opera is not very good at providing it // reliably in other circumstances. const stacktrace = ex.stacktrace || ex.stack || '' - const popSize = getPopSize(ex) + const skipLines = getSkipFirstStackStringLines(ex) try { - return defaultStackParser(stacktrace, popSize) + return defaultStackParser(stacktrace, skipLines) } catch { // no-empty } @@ -53,17 +37,47 @@ export function parseStackFrames(ex: Error & { framesToPop?: number; stacktrace? return [] } +const reactMinifiedRegexp = /Minified React error #\d+;/i + +/** + * Certain known React errors contain links that would be falsely + * parsed as frames. This function check for these errors and + * returns number of the stack string lines to skip. + */ +function getSkipFirstStackStringLines(ex: Error): number { + if (ex && reactMinifiedRegexp.test(ex.message)) { + return 1 + } + + return 0 +} + function errorPropertiesFromError(error: Error): ErrorProperties { const frames = parseStackFrames(error) return { $exception_type: error.name, - $exception_message: error.message, + $exception_message: extractMessage(error), $exception_stack_trace_raw: JSON.stringify(frames), $exception_level: 'error', } } +/** + * There are cases where stacktrace.message is an Event object + * https://github.com/getsentry/sentry-javascript/issues/1949 + * In this specific case we try to extract stacktrace.message.error.message + */ +export function extractMessage(err: Error & { message: { error?: Error } }): string { + const message = err.message + + if (message.error && typeof message.error.message === 'string') { + return message.error.message + } + + return message +} + function errorPropertiesFromString(candidate: string): ErrorProperties { return { $exception_type: 'Error', @@ -146,6 +160,7 @@ export function errorToProperties([event, source, lineno, colno, error]: ErrorEv } else { const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException') const message = domException.message ? `${name}: ${domException.message}` : name + // TODO: revisit this branch errorProperties = errorPropertiesFromString(message) errorProperties.$exception_type = isDOMError(domException) ? 'DOMError' : 'DOMException' errorProperties.$exception_message = errorProperties.$exception_message || message diff --git a/src/extensions/exception-autocapture/stack-trace.ts b/src/extensions/exception-autocapture/stack-trace.ts index d39f02a6e..219401255 100644 --- a/src/extensions/exception-autocapture/stack-trace.ts +++ b/src/extensions/exception-autocapture/stack-trace.ts @@ -2,7 +2,7 @@ // 💖open source // This was originally forked from https://github.com/csnover/TraceKit, and was largely -// re-written as part of raven - js. +// re - written as part of raven - js. // // This code was later copied to the JavaScript mono - repo and further modified and // refactored over the years. @@ -28,16 +28,9 @@ import { isUndefined } from '../../utils/type-utils' -const WEBPACK_ERROR_REGEXP = /\(error: (.*)\)/ -const STACKTRACE_FRAME_LIMIT = 50 - -const UNKNOWN_FUNCTION = '?' - -const OPERA10_PRIORITY = 10 -const OPERA11_PRIORITY = 20 -const CHROME_PRIORITY = 30 -const WINJS_PRIORITY = 40 -const GECKO_PRIORITY = 50 +export type StackParser = (stack: string, skipFirstLines?: number) => StackFrame[] +export type StackLineParserFn = (line: string) => StackFrame | undefined +export type StackLineParser = [number, StackLineParserFn] export interface StackFrame { filename?: string @@ -57,41 +50,66 @@ export interface StackFrame { debug_id?: string } +const WEBPACK_ERROR_REGEXP = /\(error: (.*)\)/ +const STRIP_FRAME_REGEXP = /captureException/ +const STACKTRACE_FRAME_LIMIT = 50 + +const UNKNOWN_FUNCTION = '?' + +const OPERA10_PRIORITY = 10 +const OPERA11_PRIORITY = 20 +const CHROME_PRIORITY = 30 +const WINJS_PRIORITY = 40 +const GECKO_PRIORITY = 50 + function createFrame(filename: string, func: string, lineno?: number, colno?: number): StackFrame { const frame: StackFrame = { filename, - function: func, + function: func === '' ? UNKNOWN_FUNCTION : func, in_app: true, // All browser frames are considered in_app } - if (!isUndefined(lineno)) { + if (isUndefined(lineno)) { frame.lineno = lineno } - if (!isUndefined(colno)) { + if (isUndefined(colno)) { frame.colno = colno } return frame } -export type StackParser = (stack: string, skipFirst?: number) => StackFrame[] -export type StackLineParserFn = (line: string) => StackFrame | undefined -export type StackLineParser = [number, StackLineParserFn] +// This regex matches frames that have no function name (ie. are at the top level of a module). +// For example "at http://localhost:5000//script.js:1:126" +// Frames _with_ function names usually look as follows: "at commitLayoutEffects (react-dom.development.js:23426:1)" +const chromeRegexNoFnName = /^\s*at (\S+?)(?::(\d+))(?::(\d+))\s*$/i -// Chromium based browsers: Chrome, Brave, new Opera, new Edge +// This regex matches all the frames that have a function name. const chromeRegex = /^\s*at (?:(.+?\)(?: \[.+\])?|.*?) ?\((?:address at )?)?(?:async )?((?:|[-a-z]+:|.*bundle|\/)?.*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i + const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/ -const chrome: StackLineParserFn = (line) => { - const parts = chromeRegex.exec(line) +// Chromium based browsers: Chrome, Brave, new Opera, new Edge +// We cannot call this variable `chrome` because it can conflict with global `chrome` variable in certain environments +// See: https://github.com/getsentry/sentry-javascript/issues/6880 +const chromeStackParserFn: StackLineParserFn = (line) => { + // If the stack line has no function name, we need to parse it differently + const noFnParts = chromeRegexNoFnName.exec(line) as null | [string, string, string, string] + + if (noFnParts) { + const [, filename, line, col] = noFnParts + return createFrame(filename, UNKNOWN_FUNCTION, +line, +col) + } + + const parts = chromeRegex.exec(line) as null | [string, string, string, string, string] if (parts) { const isEval = parts[2] && parts[2].indexOf('eval') === 0 // start of line if (isEval) { - const subMatch = chromeEvalRegex.exec(parts[2]) + const subMatch = chromeEvalRegex.exec(parts[2]) as null | [string, string, string, string] if (subMatch) { // throw out eval line/column and use top-most line/column number @@ -111,7 +129,7 @@ const chrome: StackLineParserFn = (line) => { return } -export const chromeStackLineParser: StackLineParser = [CHROME_PRIORITY, chrome] +export const chromeStackLineParser: StackLineParser = [CHROME_PRIORITY, chromeStackParserFn] // gecko regex: `(?:bundle|\d+\.js)`: `bundle` is for react native, `\d+\.js` also but specifically for ram bundles because it // generates filenames without a prefix like `file://` the filenames in the stacktrace are just 42.js @@ -121,12 +139,12 @@ const geckoREgex = const geckoEvalRegex = /(\S+) line (\d+)(?: > eval line \d+)* > eval/i const gecko: StackLineParserFn = (line) => { - const parts = geckoREgex.exec(line) + const parts = geckoREgex.exec(line) as null | [string, string, string, string, string, string] if (parts) { const isEval = parts[3] && parts[3].indexOf(' > eval') > -1 if (isEval) { - const subMatch = geckoEvalRegex.exec(parts[3]) + const subMatch = geckoEvalRegex.exec(parts[3]) as null | [string, string, string] if (subMatch) { // throw out eval line/column and use top-most line number @@ -152,7 +170,7 @@ export const geckoStackLineParser: StackLineParser = [GECKO_PRIORITY, gecko] const winjsRegex = /^\s*at (?:((?:\[object object\])?.+) )?\(?((?:[-a-z]+):.*?):(\d+)(?::(\d+))?\)?\s*$/i const winjs: StackLineParserFn = (line) => { - const parts = winjsRegex.exec(line) + const parts = winjsRegex.exec(line) as null | [string, string, string, string, string] return parts ? createFrame(parts[2], parts[1] || UNKNOWN_FUNCTION, +parts[3], parts[4] ? +parts[4] : undefined) @@ -164,7 +182,7 @@ export const winjsStackLineParser: StackLineParser = [WINJS_PRIORITY, winjs] const opera10Regex = / line (\d+).*script (?:in )?(\S+)(?:: in function (\S+))?$/i const opera10: StackLineParserFn = (line) => { - const parts = opera10Regex.exec(line) + const parts = opera10Regex.exec(line) as null | [string, string, string, string] return parts ? createFrame(parts[2], parts[3] || UNKNOWN_FUNCTION, +parts[1]) : undefined } @@ -173,39 +191,57 @@ export const opera10StackLineParser: StackLineParser = [OPERA10_PRIORITY, opera1 const opera11Regex = / line (\d+), column (\d+)\s*(?:in (?:]+)>|([^)]+))\(.*\))? in (.*):\s*$/i const opera11: StackLineParserFn = (line) => { - const parts = opera11Regex.exec(line) + const parts = opera11Regex.exec(line) as null | [string, string, string, string, string, string] return parts ? createFrame(parts[5], parts[3] || parts[4] || UNKNOWN_FUNCTION, +parts[1], +parts[2]) : undefined } export const opera11StackLineParser: StackLineParser = [OPERA11_PRIORITY, opera11] -export const defaultStackLineParsers = [chromeStackLineParser, geckoStackLineParser, winjsStackLineParser] +export const defaultStackLineParsers = [chromeStackLineParser, geckoStackLineParser] + +export const defaultStackParser = createStackParser(...defaultStackLineParsers) -export function reverse(stack: ReadonlyArray): StackFrame[] { +export function stripPostHogFramesAndReverse(stack: ReadonlyArray): StackFrame[] { if (!stack.length) { return [] } - const localStack = stack.slice(0, STACKTRACE_FRAME_LIMIT) + const localStack = Array.from(stack) + + if (/postHogWrapped/.test(getLastStackFrame(localStack).function || '')) { + localStack.pop() + } localStack.reverse() - return localStack.map((frame) => ({ + if (STRIP_FRAME_REGEXP.test(getLastStackFrame(localStack).function || '')) { + localStack.pop() + + if (STRIP_FRAME_REGEXP.test(getLastStackFrame(localStack).function || '')) { + localStack.pop() + } + } + + return localStack.slice(0, STACKTRACE_FRAME_LIMIT).map((frame) => ({ ...frame, - filename: frame.filename || localStack[localStack.length - 1].filename, - function: frame.function || '?', + filename: frame.filename || getLastStackFrame(localStack).filename, + function: frame.function || UNKNOWN_FUNCTION, })) } +function getLastStackFrame(arr: StackFrame[]): StackFrame { + return arr[arr.length - 1] || {} +} + export function createStackParser(...parsers: StackLineParser[]): StackParser { const sortedParsers = parsers.sort((a, b) => a[0] - b[0]).map((p) => p[1]) - return (stack: string, skipFirst = 0): StackFrame[] => { + return (stack: string, skipFirstLines: number = 0): StackFrame[] => { const frames: StackFrame[] = [] const lines = stack.split('\n') - for (let i = skipFirst; i < lines.length; i++) { - const line = lines[i] + for (let i = skipFirstLines; i < lines.length; i++) { + const line = lines[i] as string // Ignore lines over 1kb as they are unlikely to be stack frames. // Many of the regular expressions use backtracking which results in run time that increases exponentially with // input size. Huge strings can result in hangs/Denial of Service: @@ -238,12 +274,10 @@ export function createStackParser(...parsers: StackLineParser[]): StackParser { } } - return reverse(frames) + return stripPostHogFramesAndReverse(frames) } } -export const defaultStackParser = createStackParser(...defaultStackLineParsers) - /** * Safari web extensions, starting version unknown, can produce "frames-only" stacktraces. * What it means, is that instead of format like: @@ -270,7 +304,7 @@ const extractSafariExtensionDetails = (func: string, filename: string): [string, return isSafariExtension || isSafariWebExtension ? [ - func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION, + func.indexOf('@') !== -1 ? (func.split('@')[0] as string) : UNKNOWN_FUNCTION, isSafariExtension ? `safari-extension:${filename}` : `safari-web-extension:${filename}`, ] : [func, filename] From 86cb5201ee388a814f9754e18ba3691aaa0fe5be Mon Sep 17 00:00:00 2001 From: David Newell Date: Mon, 14 Oct 2024 16:04:07 +0100 Subject: [PATCH 2/5] some fallbacks --- .../exception-autocapture/error-conversion.ts | 58 +++++++++++-------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/src/extensions/exception-autocapture/error-conversion.ts b/src/extensions/exception-autocapture/error-conversion.ts index e5f6e8c82..e9f272f88 100644 --- a/src/extensions/exception-autocapture/error-conversion.ts +++ b/src/extensions/exception-autocapture/error-conversion.ts @@ -160,7 +160,6 @@ export function errorToProperties([event, source, lineno, colno, error]: ErrorEv } else { const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException') const message = domException.message ? `${name}: ${domException.message}` : name - // TODO: revisit this branch errorProperties = errorPropertiesFromString(message) errorProperties.$exception_type = isDOMError(domException) ? 'DOMError' : 'DOMException' errorProperties.$exception_message = errorProperties.$exception_message || message @@ -203,25 +202,7 @@ export function errorToProperties([event, source, lineno, colno, error]: ErrorEv } export function unhandledRejectionToProperties([ev]: [ev: PromiseRejectionEvent]): ErrorProperties { - // dig the object of the rejection out of known event types - let error: unknown = ev - try { - // PromiseRejectionEvents store the object of the rejection under 'reason' - // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent - if ('reason' in ev) { - error = ev.reason - } - // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents - // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into - // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec - // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and - // https://github.com/getsentry/sentry-javascript/issues/2380 - else if ('detail' in ev && 'reason' in (ev as any).detail) { - error = (ev as any).detail.reason - } - } catch { - // no-empty - } + const error = getUnhandledRejectionError(ev) // some properties are not optional but, it's useful to start off without them enforced let errorProperties: Omit & { @@ -229,6 +210,7 @@ export function unhandledRejectionToProperties([ev]: [ev: PromiseRejectionEvent] $exception_message?: string $exception_level?: string } = {} + if (isPrimitive(error)) { errorProperties = { $exception_message: `Non-Error promise rejection captured with value: ${String(error)}`, @@ -236,16 +218,46 @@ export function unhandledRejectionToProperties([ev]: [ev: PromiseRejectionEvent] } else { errorProperties = errorToProperties([error as string | Event]) } + errorProperties.$exception_handled = false return { ...errorProperties, // now we make sure the mandatory fields that were made optional are present - $exception_type: (errorProperties.$exception_type = 'UnhandledRejection'), - $exception_message: (errorProperties.$exception_message = - errorProperties.$exception_message || (ev as any).reason || String(error)), + $exception_type: errorProperties.$exception_type || 'UnhandledRejection', + $exception_message: errorProperties.$exception_message || String(error), $exception_level: isSeverityLevel(errorProperties.$exception_level) ? errorProperties.$exception_level : 'error', } } + +function getUnhandledRejectionError(error: unknown): unknown { + if (isPrimitive(error)) { + return error + } + + // dig the object of the rejection out of known event types + try { + type ErrorWithReason = { reason: unknown } + // PromiseRejectionEvents store the object of the rejection under 'reason' + // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent + if ('reason' in (error as ErrorWithReason)) { + return (error as ErrorWithReason).reason + } + + type CustomEventWithDetail = { detail: { reason: unknown } } + // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents + // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into + // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec + // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and + // https://github.com/getsentry/sentry-javascript/issues/2380 + if ('detail' in (error as CustomEventWithDetail) && 'reason' in (error as CustomEventWithDetail).detail) { + return (error as CustomEventWithDetail).detail.reason + } + } catch { + // no-empty + } + + return error +} From 316c24e10264a9a8e9d2b2d74a73dc40f91c6c54 Mon Sep 17 00:00:00 2001 From: David Newell Date: Mon, 14 Oct 2024 16:22:02 +0100 Subject: [PATCH 3/5] update stack trace --- src/extensions/exception-autocapture/stack-trace.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/extensions/exception-autocapture/stack-trace.ts b/src/extensions/exception-autocapture/stack-trace.ts index 219401255..27e47d740 100644 --- a/src/extensions/exception-autocapture/stack-trace.ts +++ b/src/extensions/exception-autocapture/stack-trace.ts @@ -201,17 +201,13 @@ export const defaultStackLineParsers = [chromeStackLineParser, geckoStackLinePar export const defaultStackParser = createStackParser(...defaultStackLineParsers) -export function stripPostHogFramesAndReverse(stack: ReadonlyArray): StackFrame[] { +export function reverseAndStripFrames(stack: ReadonlyArray): StackFrame[] { if (!stack.length) { return [] } const localStack = Array.from(stack) - if (/postHogWrapped/.test(getLastStackFrame(localStack).function || '')) { - localStack.pop() - } - localStack.reverse() if (STRIP_FRAME_REGEXP.test(getLastStackFrame(localStack).function || '')) { @@ -274,7 +270,7 @@ export function createStackParser(...parsers: StackLineParser[]): StackParser { } } - return stripPostHogFramesAndReverse(frames) + return reverseAndStripFrames(frames) } } From 3ce794b55b663d30ae612f019f21c7fb681ae189 Mon Sep 17 00:00:00 2001 From: David Newell Date: Mon, 14 Oct 2024 18:59:38 +0100 Subject: [PATCH 4/5] reset --- src/extensions/exception-autocapture/error-conversion.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extensions/exception-autocapture/error-conversion.ts b/src/extensions/exception-autocapture/error-conversion.ts index e9f272f88..722e4ac2c 100644 --- a/src/extensions/exception-autocapture/error-conversion.ts +++ b/src/extensions/exception-autocapture/error-conversion.ts @@ -224,7 +224,7 @@ export function unhandledRejectionToProperties([ev]: [ev: PromiseRejectionEvent] return { ...errorProperties, // now we make sure the mandatory fields that were made optional are present - $exception_type: errorProperties.$exception_type || 'UnhandledRejection', + $exception_type: (errorProperties.$exception_type = 'UnhandledRejection'), $exception_message: errorProperties.$exception_message || String(error), $exception_level: isSeverityLevel(errorProperties.$exception_level) ? errorProperties.$exception_level From 6a28ff22d1b34828ed7df5450e56e0d79cdacb9c Mon Sep 17 00:00:00 2001 From: David Newell Date: Tue, 15 Oct 2024 10:47:09 +0100 Subject: [PATCH 5/5] fix merge --- .../exception-autocapture/error-conversion.ts | 38 ++++++++++++++++++- .../exception-autocapture/stack-trace.ts | 6 +-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/extensions/exception-autocapture/error-conversion.ts b/src/extensions/exception-autocapture/error-conversion.ts index 29f7e4944..f4997690d 100644 --- a/src/extensions/exception-autocapture/error-conversion.ts +++ b/src/extensions/exception-autocapture/error-conversion.ts @@ -11,8 +11,44 @@ import { import { defaultStackParser, StackFrame } from './stack-trace' import { isEmptyString, isString, isUndefined } from '../../utils/type-utils' -import { ErrorEventArgs, ErrorMetadata, ErrorProperties, SeverityLevel, severityLevels } from '../../types' +import { ErrorEventArgs, ErrorMetadata, SeverityLevel, severityLevels } from '../../types' +export interface ErrorProperties { + $exception_list: Exception[] + $exception_level?: SeverityLevel + $exception_DOMException_code?: string + $exception_personURL?: string +} + +export interface Exception { + type?: string + value?: string + mechanism?: { + /** + * In theory, whether or not the exception has been handled by the user. In practice, whether or not we see it before + * it hits the global error/rejection handlers, whether through explicit handling by the user or auto instrumentation. + */ + handled?: boolean + type?: string + source?: string + /** + * True when `captureException` is called with anything other than an instance of `Error` (or, in the case of browser, + * an instance of `ErrorEvent`, `DOMError`, or `DOMException`). causing us to create a synthetic error in an attempt + * to recreate the stacktrace. + */ + synthetic?: boolean + } + module?: string + thread_id?: number + stacktrace?: { + frames?: StackFrame[] + } +} + +export interface ErrorConversions { + errorToProperties: (args: ErrorEventArgs, metadata?: ErrorMetadata) => ErrorProperties + unhandledRejectionToProperties: (args: [ev: PromiseRejectionEvent]) => ErrorProperties +} /** * based on the very wonderful MIT licensed Sentry SDK */ diff --git a/src/extensions/exception-autocapture/stack-trace.ts b/src/extensions/exception-autocapture/stack-trace.ts index 27e47d740..f09127713 100644 --- a/src/extensions/exception-autocapture/stack-trace.ts +++ b/src/extensions/exception-autocapture/stack-trace.ts @@ -2,7 +2,7 @@ // 💖open source // This was originally forked from https://github.com/csnover/TraceKit, and was largely -// re - written as part of raven - js. +// re-written as part of raven - js. // // This code was later copied to the JavaScript mono - repo and further modified and // refactored over the years. @@ -69,11 +69,11 @@ function createFrame(filename: string, func: string, lineno?: number, colno?: nu in_app: true, // All browser frames are considered in_app } - if (isUndefined(lineno)) { + if (!isUndefined(lineno)) { frame.lineno = lineno } - if (isUndefined(colno)) { + if (!isUndefined(colno)) { frame.colno = colno }