From f6d62d807bf3c6b92ac6cd7b3e7197de0c027918 Mon Sep 17 00:00:00 2001 From: David Newell Date: Tue, 15 Oct 2024 11:14:30 +0100 Subject: [PATCH] chore: improve exception autocapture (#1466) --- .../exception-autocapture/error-conversion.ts | 105 +++++++++++------- .../exception-autocapture/stack-trace.ts | 104 +++++++++++------ 2 files changed, 133 insertions(+), 76 deletions(-) diff --git a/src/extensions/exception-autocapture/error-conversion.ts b/src/extensions/exception-autocapture/error-conversion.ts index 9d2c1e19c..f4997690d 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, ErrorMetadata, SeverityLevel, severityLevels } from '../../types' export interface ErrorProperties { @@ -49,7 +49,6 @@ export interface ErrorConversions { errorToProperties: (args: ErrorEventArgs, metadata?: ErrorMetadata) => ErrorProperties unhandledRejectionToProperties: (args: [ev: PromiseRejectionEvent]) => ErrorProperties } - /** * based on the very wonderful MIT licensed Sentry SDK */ @@ -57,32 +56,16 @@ export interface ErrorConversions { 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 } @@ -90,6 +73,21 @@ 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, metadata?: ErrorMetadata): ErrorProperties { const frames = parseStackFrames(error) @@ -97,7 +95,9 @@ function errorPropertiesFromError(error: Error, metadata?: ErrorMetadata): Error const synthetic = metadata?.synthetic ?? false const exceptionType = metadata?.overrideExceptionType ? metadata.overrideExceptionType : error.name - const exceptionMessage = metadata?.overrideExceptionMessage ? metadata.overrideExceptionMessage : error.message + const exceptionMessage = metadata?.overrideExceptionMessage + ? metadata.overrideExceptionMessage + : extractMessage(error) return { $exception_list: [ @@ -117,6 +117,21 @@ function errorPropertiesFromError(error: Error, metadata?: ErrorMetadata): 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, metadata?: ErrorMetadata): ErrorProperties { // Defaults for metadata are based on what the error candidate is. const handled = metadata?.handled ?? true @@ -265,37 +280,49 @@ export function errorToProperties( } export function unhandledRejectionToProperties([ev]: [ev: PromiseRejectionEvent]): ErrorProperties { + const error = getUnhandledRejectionError(ev) + + if (isPrimitive(error)) { + return errorPropertiesFromString(`Non-Error promise rejection captured with value: ${String(error)}`, { + handled: false, + synthetic: false, + overrideExceptionType: 'UnhandledRejection', + }) + } + + return errorToProperties([error as string | Event], { + handled: false, + overrideExceptionType: 'UnhandledRejection', + defaultExceptionMessage: String(error), + }) +} + +function getUnhandledRejectionError(error: unknown): unknown { + if (isPrimitive(error)) { + return error + } + // dig the object of the rejection out of known event types - let error: unknown = ev 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 ev) { - error = ev.reason + 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 - else if ('detail' in ev && 'reason' in (ev as any).detail) { - error = (ev as any).detail.reason + if ('detail' in (error as CustomEventWithDetail) && 'reason' in (error as CustomEventWithDetail).detail) { + return (error as CustomEventWithDetail).detail.reason } } catch { // no-empty } - if (isPrimitive(error)) { - return errorPropertiesFromString(`Non-Error promise rejection captured with value: ${String(error)}`, { - handled: false, - synthetic: false, - overrideExceptionType: 'UnhandledRejection', - }) - } else { - return errorToProperties([error as string | Event], { - handled: false, - overrideExceptionType: 'UnhandledRejection', - defaultExceptionMessage: (ev as any).reason || String(error), - }) - } + return error } diff --git a/src/extensions/exception-autocapture/stack-trace.ts b/src/extensions/exception-autocapture/stack-trace.ts index d39f02a6e..f09127713 100644 --- a/src/extensions/exception-autocapture/stack-trace.ts +++ b/src/extensions/exception-autocapture/stack-trace.ts @@ -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,10 +50,22 @@ 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 } @@ -75,23 +80,36 @@ function createFrame(filename: string, func: string, lineno?: number, colno?: nu 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,53 @@ 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 function reverse(stack: ReadonlyArray): StackFrame[] { +export const defaultStackParser = createStackParser(...defaultStackLineParsers) + +export function reverseAndStripFrames(stack: ReadonlyArray): StackFrame[] { if (!stack.length) { return [] } - const localStack = stack.slice(0, STACKTRACE_FRAME_LIMIT) + const localStack = Array.from(stack) 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 +270,10 @@ export function createStackParser(...parsers: StackLineParser[]): StackParser { } } - return reverse(frames) + return reverseAndStripFrames(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 +300,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]