Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
ref(browser): Split stack line parsers into individual functions and simplify further#4555
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
0ec6249
Refactor tests and minor improvements
timfish 5563471
Remove TraceKitStackFrame
timfish 4db0290
Remove undefined
timfish 8e09b5e
Merge remote-tracking branch 'upstream/master' into remove-tracekitst…
timfish 87c5628
Merge opera parser to slim down code
timfish 245eee9
Lil tidy
timfish 8723236
split into functions
timfish ee31a1a
More simplify
timfish 6627be7
Minimise fields
timfish e45decc
Merge remote-tracking branch 'upstream/master' into sep-parsers
timfish 328118b
Re order so we dont break opera!
timfish 3c1e7c2
Move the regex
timfish 98be3a1
Move frame reverse into parser and reverse all test frames 😭
timfish 11a4318
simplify pop
timfish 1c40c42
Couple more simplifications
timfish 24cc5a4
Merge remote-tracking branch 'upstream/master' into sep-parsers
timfish 2bee1b4
Remove useless else
timfish 24a18f3
Move `in_app` to initial declaration
timfish 426828e
Fix bad code
timfish cb8af0d
Merge remote-tracking branch 'upstream/master' into sep-parsers
timfish File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,21 @@ | ||
| /* eslint-disable @typescript-eslint/no-unsafe-member-access */ | ||
| import { Event, Exception, StackFrame } from '@sentry/types'; | ||
| import { extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils'; | ||
| import { createStackParser, extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils'; | ||
| import { computeStackTrace, StackTrace as TraceKitStackTrace } from './tracekit'; | ||
| const STACKTRACE_LIMIT = 50; | ||
| import { chrome, gecko, opera10, opera11, winjs } from './stack-parsers'; | ||
| /** | ||
| * This function creates an exception from an TraceKitStackTrace | ||
| * @param stacktrace TraceKitStackTrace that will be converted to an exception | ||
| * @hidden | ||
| */ | ||
| export function exceptionFromStacktrace(stacktrace: TraceKitStackTrace): Exception { | ||
| const frames = prepareFramesForEvent(stacktrace.stack); | ||
| export function exceptionFromError(ex: Error): Exception { | ||
| // Get the frames first since Opera can lose the stack if we touch anything else first | ||
| const frames = parseStackFrames(ex); | ||
| const exception: Exception = { | ||
| type: stacktrace.name, | ||
| value: stacktrace.message, | ||
| type: ex && ex.name, | ||
| value: extractMessage(ex), | ||
| }; | ||
| if (frames && frames.length) { | ||
| @@ -54,10 +54,8 @@ export function eventFromPlainObject( | ||
| }; | ||
| if (syntheticException) { | ||
| const stacktrace = computeStackTrace(syntheticException); | ||
| const frames = prepareFramesForEvent(stacktrace.stack); | ||
| event.stacktrace = { | ||
| frames, | ||
| frames: parseStackFrames(syntheticException), | ||
| }; | ||
| } | ||
| @@ -67,48 +65,63 @@ export function eventFromPlainObject( | ||
| /** | ||
| * @hidden | ||
| */ | ||
| export function eventFromStacktrace(stacktrace: TraceKitStackTrace): Event { | ||
| const exception = exceptionFromStacktrace(stacktrace); | ||
| export function eventFromError(ex: Error): Event { | ||
| return { | ||
| exception: { | ||
| values: [exception], | ||
| values: [exceptionFromError(ex)], | ||
| }, | ||
| }; | ||
| } | ||
| /** | ||
| * @hidden | ||
| */ | ||
| export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] { | ||
| if (!stack.length) { | ||
| return []; | ||
| /** Parses stack frames from an error */ | ||
| 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); | ||
| try { | ||
| // The order of the parsers in important | ||
| return createStackParser(opera10, opera11, chrome, winjs, gecko)(stacktrace, popSize); | ||
| } catch (e) { | ||
| // no-empty | ||
| } | ||
| let localStack = stack; | ||
| return []; | ||
| } | ||
| const firstFrameFunction = localStack[0].function || ''; | ||
| const lastFrameFunction = localStack[localStack.length - 1].function || ''; | ||
| // Based on our own mapping pattern - https://github.com/getsentry/sentry/blob/9f08305e09866c8bd6d0c24f5b0aabdd7dd6c59c/src/sentry/lang/javascript/errormapping.py#L83-L108 | ||
| const reactMinifiedRegexp = /Minified React error #\d+;/i; | ||
| // If stack starts with one of our API calls, remove it (starts, meaning it's the top of the stack - aka last call) | ||
| if (firstFrameFunction.indexOf('captureMessage') !== -1 || firstFrameFunction.indexOf('captureException') !== -1) { | ||
| localStack = localStack.slice(1); | ||
| } | ||
| function getPopSize(ex: Error & { framesToPop?: number }): number { | ||
| if (ex) { | ||
| if (typeof ex.framesToPop === 'number') { | ||
| return ex.framesToPop; | ||
| } | ||
| // If stack ends with one of our internal API calls, remove it (ends, meaning it's the bottom of the stack - aka top-most call) | ||
| if (lastFrameFunction.indexOf('sentryWrapped') !== -1) { | ||
| localStack = localStack.slice(0, -1); | ||
| if (reactMinifiedRegexp.test(ex.message)) { | ||
| return 1; | ||
timfish marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| } | ||
| // The frame where the crash happened, should be the last entry in the array | ||
| return localStack | ||
| .slice(0, STACKTRACE_LIMIT) | ||
| .map(frame => ({ | ||
| filename: frame.filename || localStack[0].filename, | ||
| function: frame.function || '?', | ||
| lineno: frame.lineno, | ||
| colno: frame.colno, | ||
| in_app: true, | ||
| })) | ||
| .reverse(); | ||
| return 0; | ||
| } | ||
| /** | ||
| * 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 | ||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function extractMessage(ex: any): string { | ||
| const message = ex && ex.message; | ||
| if (!message) { | ||
| return 'No error message'; | ||
| } | ||
| if (message.error && typeof message.error.message === 'string') { | ||
| return message.error.message; | ||
| } | ||
| return message; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import { StackFrame } from '@sentry/types'; | ||
| import { StackLineParser } from '@sentry/utils'; | ||
| // global reference to slice | ||
| const UNKNOWN_FUNCTION = '?'; | ||
| function createFrame(filename: string, func: string, lineno?: number, colno?: number): StackFrame { | ||
| const frame: StackFrame = { | ||
| filename, | ||
| function: func, | ||
| // All browser frames are considered in_app | ||
| in_app: true, | ||
| }; | ||
| if (lineno !== undefined) { | ||
| frame.lineno = lineno; | ||
| } | ||
| if (colno !== undefined) { | ||
| frame.colno = colno; | ||
| } | ||
| return frame; | ||
| } | ||
| // Chromium based browsers: Chrome, Brave, new Opera, new Edge | ||
| const chromeRegex = | ||
| /^\s*at (?:(.*?) ?\((?:address at )?)?((?:file|https?|blob|chrome-extension|address|native|eval|webpack|<anonymous>|[-a-z]+:|.*bundle|\/).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i; | ||
| const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/; | ||
| export const chrome: StackLineParser = line => { | ||
| const parts = chromeRegex.exec(line); | ||
| if (parts) { | ||
| const isEval = parts[2] && parts[2].indexOf('eval') === 0; // start of line | ||
| if (isEval) { | ||
| const subMatch = chromeEvalRegex.exec(parts[2]); | ||
| if (subMatch) { | ||
| // throw out eval line/column and use top-most line/column number | ||
| parts[2] = subMatch[1]; // url | ||
| parts[3] = subMatch[2]; // line | ||
| parts[4] = subMatch[3]; // column | ||
| } | ||
| } | ||
| // Kamil: One more hack won't hurt us right? Understanding and adding more rules on top of these regexps right now | ||
| // would be way too time consuming. (TODO: Rewrite whole RegExp to be more readable) | ||
| const [func, filename] = extractSafariExtensionDetails(parts[1] || UNKNOWN_FUNCTION, parts[2]); | ||
| return createFrame(filename, func, parts[3] ? +parts[3] : undefined, parts[4] ? +parts[4] : undefined); | ||
| } | ||
| return; | ||
| }; | ||
| // 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 | ||
| // We need this specific case for now because we want no other regex to match. | ||
| const geckoREgex = | ||
| /^\s*(.*?)(?:\((.*?)\))?(?:^|@)?((?:file|https?|blob|chrome|webpack|resource|moz-extension|capacitor).*?:\/.*?|\[native code\]|[^@]*(?:bundle|\d+\.js)|\/[\w\-. /=]+)(?::(\d+))?(?::(\d+))?\s*$/i; | ||
| const geckoEvalRegex = /(\S+) line (\d+)(?: > eval line \d+)* > eval/i; | ||
| export const gecko: StackLineParser = line => { | ||
| const parts = geckoREgex.exec(line); | ||
| if (parts) { | ||
| const isEval = parts[3] && parts[3].indexOf(' > eval') > -1; | ||
| if (isEval) { | ||
| const subMatch = geckoEvalRegex.exec(parts[3]); | ||
| if (subMatch) { | ||
| // throw out eval line/column and use top-most line number | ||
| parts[1] = parts[1] || `eval`; | ||
| parts[3] = subMatch[1]; | ||
| parts[4] = subMatch[2]; | ||
| parts[5] = ''; // no column when eval | ||
| } | ||
| } | ||
| let filename = parts[3]; | ||
| let func = parts[1] || UNKNOWN_FUNCTION; | ||
| [func, filename] = extractSafariExtensionDetails(func, filename); | ||
| return createFrame(filename, func, parts[4] ? +parts[4] : undefined, parts[5] ? +parts[5] : undefined); | ||
| } | ||
| return; | ||
| }; | ||
| const winjsRegex = | ||
| /^\s*at (?:((?:\[object object\])?.+) )?\(?((?:file|ms-appx|https?|webpack|blob):.*?):(\d+)(?::(\d+))?\)?\s*$/i; | ||
| export const winjs: StackLineParser = line => { | ||
| const parts = winjsRegex.exec(line); | ||
| return parts | ||
| ? createFrame(parts[2], parts[1] || UNKNOWN_FUNCTION, +parts[3], parts[4] ? +parts[4] : undefined) | ||
| : undefined; | ||
| }; | ||
| const opera10Regex = / line (\d+).*script (?:in )?(\S+)(?:: in function (\S+))?$/i; | ||
| export const opera10: StackLineParser = line => { | ||
| const parts = opera10Regex.exec(line); | ||
| return parts ? createFrame(parts[2], parts[3] || UNKNOWN_FUNCTION, +parts[1]) : undefined; | ||
| }; | ||
| const opera11Regex = | ||
| / line (\d+), column (\d+)\s*(?:in (?:<anonymous function: ([^>]+)>|([^)]+))\(.*\))? in (.*):\s*$/i; | ||
| export const opera11: StackLineParser = line => { | ||
| const parts = opera11Regex.exec(line); | ||
| return parts ? createFrame(parts[5], parts[3] || parts[4] || UNKNOWN_FUNCTION, +parts[1], +parts[2]) : undefined; | ||
| }; | ||
| /** | ||
| * Safari web extensions, starting version unknown, can produce "frames-only" stacktraces. | ||
| * What it means, is that instead of format like: | ||
| * | ||
| * Error: wat | ||
| * at function@url:row:col | ||
| * at function@url:row:col | ||
| * at function@url:row:col | ||
| * | ||
| * it produces something like: | ||
| * | ||
| * function@url:row:col | ||
| * function@url:row:col | ||
| * function@url:row:col | ||
| * | ||
| * Because of that, it won't be captured by `chrome` RegExp and will fall into `Gecko` branch. | ||
| * This function is extracted so that we can use it in both places without duplicating the logic. | ||
| * Unfortunately "just" changing RegExp is too complicated now and making it pass all tests | ||
| * and fix this case seems like an impossible, or at least way too time-consuming task. | ||
| */ | ||
| const extractSafariExtensionDetails = (func: string, filename: string): [string, string] => { | ||
| const isSafariExtension = func.indexOf('safari-extension') !== -1; | ||
| const isSafariWebExtension = func.indexOf('safari-web-extension') !== -1; | ||
| return isSafariExtension || isSafariWebExtension | ||
| ? [ | ||
| func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION, | ||
| isSafariExtension ? `safari-extension:${filename}` : `safari-web-extension:${filename}`, | ||
| ] | ||
| : [func, filename]; | ||
| }; |
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.