Skip to content

Commit 858c2f7

Browse files
authored
ref(browser): Split stack line parsers into individual functions and simplify further (#4555)
- Converts parsers into individual functions now in `stack-parsers.ts` - Removes `TraceKitStackTrace` and instead just returns `StackFrame[]` - Replaces `exceptionFromStacktrace(computeStackTrace(e))` with `exceptionFromError(e))` - Replaces `eventFromStacktrace(computeStackTrace(e))` with `eventFromError(e))` - Changes tests to test against a full `Exception` which meant frames needed reversing and `in_app` adding - Moves `createStackParser` and `StackLineParser` type to `@sentry/utils` where there was already a `stacktrace.ts` - Renamed `prepareFramesForEvent` to `stripSentryFramesAndReverse` and moved to utils too. This is nearly identical to the node.js code and means the parser now returns frames ready to go into `Exception` - Moved `in_app` to `createFrame` in stack parsers since everything browser is considered `in_app` - Simplified frame popping so it occurs before parsing. ie. `stack.split('\n').slice(skipFirst)`
1 parent af9d08f commit 858c2f7

15 files changed

+1306
-1020
lines changed

packages/browser/src/eventbuilder.ts

+5-12
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ import {
1111
resolvedSyncPromise,
1212
} from '@sentry/utils';
1313

14-
import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
15-
import { computeStackTrace } from './tracekit';
14+
import { eventFromError, eventFromPlainObject, parseStackFrames } from './parsers';
1615

1716
/**
1817
* Creates an {@link Event} from all inputs to `captureException` and non-primitive inputs to `captureMessage`.
@@ -68,10 +67,7 @@ export function eventFromUnknownInput(
6867
if (isErrorEvent(exception as ErrorEvent) && (exception as ErrorEvent).error) {
6968
// If it is an ErrorEvent with `error` property, extract it to get actual Error
7069
const errorEvent = exception as ErrorEvent;
71-
// eslint-disable-next-line no-param-reassign
72-
exception = errorEvent.error;
73-
event = eventFromStacktrace(computeStackTrace(exception as Error));
74-
return event;
70+
return eventFromError(errorEvent.error as Error);
7571
}
7672

7773
// If it is a `DOMError` (which is a legacy API, but still supported in some browsers) then we just extract the name
@@ -85,7 +81,7 @@ export function eventFromUnknownInput(
8581
const domException = exception as DOMException;
8682

8783
if ('stack' in (exception as Error)) {
88-
event = eventFromStacktrace(computeStackTrace(exception as Error));
84+
event = eventFromError(exception as Error);
8985
} else {
9086
const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException');
9187
const message = domException.message ? `${name}: ${domException.message}` : name;
@@ -100,8 +96,7 @@ export function eventFromUnknownInput(
10096
}
10197
if (isError(exception as Error)) {
10298
// we have a real Error object, do nothing
103-
event = eventFromStacktrace(computeStackTrace(exception as Error));
104-
return event;
99+
return eventFromError(exception as Error);
105100
}
106101
if (isPlainObject(exception) || isEvent(exception)) {
107102
// If it's a plain object or an instance of `Event` (the built-in JS kind, not this SDK's `Event` type), serialize
@@ -148,10 +143,8 @@ export function eventFromString(
148143
};
149144

150145
if (options.attachStacktrace && syntheticException) {
151-
const stacktrace = computeStackTrace(syntheticException);
152-
const frames = prepareFramesForEvent(stacktrace.stack);
153146
event.stacktrace = {
154-
frames,
147+
frames: parseStackFrames(syntheticException),
155148
};
156149
}
157150

packages/browser/src/integrations/linkederrors.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
22
import { Event, EventHint, Exception, ExtendedError, Integration } from '@sentry/types';
33
import { isInstanceOf } from '@sentry/utils';
44

5-
import { exceptionFromStacktrace } from '../parsers';
6-
import { computeStackTrace } from '../tracekit';
5+
import { exceptionFromError } from '../parsers';
76

87
const DEFAULT_KEY = 'cause';
98
const DEFAULT_LIMIT = 5;
@@ -73,7 +72,6 @@ export function _walkErrorTree(limit: number, error: ExtendedError, key: string,
7372
if (!isInstanceOf(error[key], Error) || stack.length + 1 >= limit) {
7473
return stack;
7574
}
76-
const stacktrace = computeStackTrace(error[key]);
77-
const exception = exceptionFromStacktrace(stacktrace);
75+
const exception = exceptionFromError(error[key]);
7876
return _walkErrorTree(limit, error[key], key, [exception, ...stack]);
7977
}

packages/browser/src/parsers.ts

+55-42
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
1+
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
12
import { Event, Exception, StackFrame } from '@sentry/types';
2-
import { extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils';
3+
import { createStackParser, extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils';
34

4-
import { computeStackTrace, StackTrace as TraceKitStackTrace } from './tracekit';
5-
6-
const STACKTRACE_LIMIT = 50;
5+
import { chrome, gecko, opera10, opera11, winjs } from './stack-parsers';
76

87
/**
98
* This function creates an exception from an TraceKitStackTrace
109
* @param stacktrace TraceKitStackTrace that will be converted to an exception
1110
* @hidden
1211
*/
13-
export function exceptionFromStacktrace(stacktrace: TraceKitStackTrace): Exception {
14-
const frames = prepareFramesForEvent(stacktrace.stack);
12+
export function exceptionFromError(ex: Error): Exception {
13+
// Get the frames first since Opera can lose the stack if we touch anything else first
14+
const frames = parseStackFrames(ex);
1515

1616
const exception: Exception = {
17-
type: stacktrace.name,
18-
value: stacktrace.message,
17+
type: ex && ex.name,
18+
value: extractMessage(ex),
1919
};
2020

2121
if (frames && frames.length) {
@@ -54,10 +54,8 @@ export function eventFromPlainObject(
5454
};
5555

5656
if (syntheticException) {
57-
const stacktrace = computeStackTrace(syntheticException);
58-
const frames = prepareFramesForEvent(stacktrace.stack);
5957
event.stacktrace = {
60-
frames,
58+
frames: parseStackFrames(syntheticException),
6159
};
6260
}
6361

@@ -67,48 +65,63 @@ export function eventFromPlainObject(
6765
/**
6866
* @hidden
6967
*/
70-
export function eventFromStacktrace(stacktrace: TraceKitStackTrace): Event {
71-
const exception = exceptionFromStacktrace(stacktrace);
72-
68+
export function eventFromError(ex: Error): Event {
7369
return {
7470
exception: {
75-
values: [exception],
71+
values: [exceptionFromError(ex)],
7672
},
7773
};
7874
}
7975

80-
/**
81-
* @hidden
82-
*/
83-
export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] {
84-
if (!stack.length) {
85-
return [];
76+
/** Parses stack frames from an error */
77+
export function parseStackFrames(ex: Error & { framesToPop?: number; stacktrace?: string }): StackFrame[] {
78+
// Access and store the stacktrace property before doing ANYTHING
79+
// else to it because Opera is not very good at providing it
80+
// reliably in other circumstances.
81+
const stacktrace = ex.stacktrace || ex.stack || '';
82+
83+
const popSize = getPopSize(ex);
84+
85+
try {
86+
// The order of the parsers in important
87+
return createStackParser(opera10, opera11, chrome, winjs, gecko)(stacktrace, popSize);
88+
} catch (e) {
89+
// no-empty
8690
}
8791

88-
let localStack = stack;
92+
return [];
93+
}
8994

90-
const firstFrameFunction = localStack[0].function || '';
91-
const lastFrameFunction = localStack[localStack.length - 1].function || '';
95+
// Based on our own mapping pattern - https://github.com/getsentry/sentry/blob/9f08305e09866c8bd6d0c24f5b0aabdd7dd6c59c/src/sentry/lang/javascript/errormapping.py#L83-L108
96+
const reactMinifiedRegexp = /Minified React error #\d+;/i;
9297

93-
// If stack starts with one of our API calls, remove it (starts, meaning it's the top of the stack - aka last call)
94-
if (firstFrameFunction.indexOf('captureMessage') !== -1 || firstFrameFunction.indexOf('captureException') !== -1) {
95-
localStack = localStack.slice(1);
96-
}
98+
function getPopSize(ex: Error & { framesToPop?: number }): number {
99+
if (ex) {
100+
if (typeof ex.framesToPop === 'number') {
101+
return ex.framesToPop;
102+
}
97103

98-
// 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)
99-
if (lastFrameFunction.indexOf('sentryWrapped') !== -1) {
100-
localStack = localStack.slice(0, -1);
104+
if (reactMinifiedRegexp.test(ex.message)) {
105+
return 1;
106+
}
101107
}
102108

103-
// The frame where the crash happened, should be the last entry in the array
104-
return localStack
105-
.slice(0, STACKTRACE_LIMIT)
106-
.map(frame => ({
107-
filename: frame.filename || localStack[0].filename,
108-
function: frame.function || '?',
109-
lineno: frame.lineno,
110-
colno: frame.colno,
111-
in_app: true,
112-
}))
113-
.reverse();
109+
return 0;
110+
}
111+
112+
/**
113+
* There are cases where stacktrace.message is an Event object
114+
* https://github.com/getsentry/sentry-javascript/issues/1949
115+
* In this specific case we try to extract stacktrace.message.error.message
116+
*/
117+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
118+
function extractMessage(ex: any): string {
119+
const message = ex && ex.message;
120+
if (!message) {
121+
return 'No error message';
122+
}
123+
if (message.error && typeof message.error.message === 'string') {
124+
return message.error.message;
125+
}
126+
return message;
114127
}

packages/browser/src/stack-parsers.ts

+148
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { StackFrame } from '@sentry/types';
2+
import { StackLineParser } from '@sentry/utils';
3+
4+
// global reference to slice
5+
const UNKNOWN_FUNCTION = '?';
6+
7+
function createFrame(filename: string, func: string, lineno?: number, colno?: number): StackFrame {
8+
const frame: StackFrame = {
9+
filename,
10+
function: func,
11+
// All browser frames are considered in_app
12+
in_app: true,
13+
};
14+
15+
if (lineno !== undefined) {
16+
frame.lineno = lineno;
17+
}
18+
19+
if (colno !== undefined) {
20+
frame.colno = colno;
21+
}
22+
23+
return frame;
24+
}
25+
26+
// Chromium based browsers: Chrome, Brave, new Opera, new Edge
27+
const chromeRegex =
28+
/^\s*at (?:(.*?) ?\((?:address at )?)?((?:file|https?|blob|chrome-extension|address|native|eval|webpack|<anonymous>|[-a-z]+:|.*bundle|\/).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i;
29+
const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/;
30+
31+
export const chrome: StackLineParser = line => {
32+
const parts = chromeRegex.exec(line);
33+
34+
if (parts) {
35+
const isEval = parts[2] && parts[2].indexOf('eval') === 0; // start of line
36+
37+
if (isEval) {
38+
const subMatch = chromeEvalRegex.exec(parts[2]);
39+
40+
if (subMatch) {
41+
// throw out eval line/column and use top-most line/column number
42+
parts[2] = subMatch[1]; // url
43+
parts[3] = subMatch[2]; // line
44+
parts[4] = subMatch[3]; // column
45+
}
46+
}
47+
48+
// Kamil: One more hack won't hurt us right? Understanding and adding more rules on top of these regexps right now
49+
// would be way too time consuming. (TODO: Rewrite whole RegExp to be more readable)
50+
const [func, filename] = extractSafariExtensionDetails(parts[1] || UNKNOWN_FUNCTION, parts[2]);
51+
52+
return createFrame(filename, func, parts[3] ? +parts[3] : undefined, parts[4] ? +parts[4] : undefined);
53+
}
54+
55+
return;
56+
};
57+
58+
// gecko regex: `(?:bundle|\d+\.js)`: `bundle` is for react native, `\d+\.js` also but specifically for ram bundles because it
59+
// generates filenames without a prefix like `file://` the filenames in the stacktrace are just 42.js
60+
// We need this specific case for now because we want no other regex to match.
61+
const geckoREgex =
62+
/^\s*(.*?)(?:\((.*?)\))?(?:^|@)?((?:file|https?|blob|chrome|webpack|resource|moz-extension|capacitor).*?:\/.*?|\[native code\]|[^@]*(?:bundle|\d+\.js)|\/[\w\-. /=]+)(?::(\d+))?(?::(\d+))?\s*$/i;
63+
const geckoEvalRegex = /(\S+) line (\d+)(?: > eval line \d+)* > eval/i;
64+
65+
export const gecko: StackLineParser = line => {
66+
const parts = geckoREgex.exec(line);
67+
68+
if (parts) {
69+
const isEval = parts[3] && parts[3].indexOf(' > eval') > -1;
70+
if (isEval) {
71+
const subMatch = geckoEvalRegex.exec(parts[3]);
72+
73+
if (subMatch) {
74+
// throw out eval line/column and use top-most line number
75+
parts[1] = parts[1] || `eval`;
76+
parts[3] = subMatch[1];
77+
parts[4] = subMatch[2];
78+
parts[5] = ''; // no column when eval
79+
}
80+
}
81+
82+
let filename = parts[3];
83+
let func = parts[1] || UNKNOWN_FUNCTION;
84+
[func, filename] = extractSafariExtensionDetails(func, filename);
85+
86+
return createFrame(filename, func, parts[4] ? +parts[4] : undefined, parts[5] ? +parts[5] : undefined);
87+
}
88+
89+
return;
90+
};
91+
92+
const winjsRegex =
93+
/^\s*at (?:((?:\[object object\])?.+) )?\(?((?:file|ms-appx|https?|webpack|blob):.*?):(\d+)(?::(\d+))?\)?\s*$/i;
94+
95+
export const winjs: StackLineParser = line => {
96+
const parts = winjsRegex.exec(line);
97+
98+
return parts
99+
? createFrame(parts[2], parts[1] || UNKNOWN_FUNCTION, +parts[3], parts[4] ? +parts[4] : undefined)
100+
: undefined;
101+
};
102+
103+
const opera10Regex = / line (\d+).*script (?:in )?(\S+)(?:: in function (\S+))?$/i;
104+
105+
export const opera10: StackLineParser = line => {
106+
const parts = opera10Regex.exec(line);
107+
return parts ? createFrame(parts[2], parts[3] || UNKNOWN_FUNCTION, +parts[1]) : undefined;
108+
};
109+
110+
const opera11Regex =
111+
/ line (\d+), column (\d+)\s*(?:in (?:<anonymous function: ([^>]+)>|([^)]+))\(.*\))? in (.*):\s*$/i;
112+
113+
export const opera11: StackLineParser = line => {
114+
const parts = opera11Regex.exec(line);
115+
return parts ? createFrame(parts[5], parts[3] || parts[4] || UNKNOWN_FUNCTION, +parts[1], +parts[2]) : undefined;
116+
};
117+
118+
/**
119+
* Safari web extensions, starting version unknown, can produce "frames-only" stacktraces.
120+
* What it means, is that instead of format like:
121+
*
122+
* Error: wat
123+
* at function@url:row:col
124+
* at function@url:row:col
125+
* at function@url:row:col
126+
*
127+
* it produces something like:
128+
*
129+
* function@url:row:col
130+
* function@url:row:col
131+
* function@url:row:col
132+
*
133+
* Because of that, it won't be captured by `chrome` RegExp and will fall into `Gecko` branch.
134+
* This function is extracted so that we can use it in both places without duplicating the logic.
135+
* Unfortunately "just" changing RegExp is too complicated now and making it pass all tests
136+
* and fix this case seems like an impossible, or at least way too time-consuming task.
137+
*/
138+
const extractSafariExtensionDetails = (func: string, filename: string): [string, string] => {
139+
const isSafariExtension = func.indexOf('safari-extension') !== -1;
140+
const isSafariWebExtension = func.indexOf('safari-web-extension') !== -1;
141+
142+
return isSafariExtension || isSafariWebExtension
143+
? [
144+
func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION,
145+
isSafariExtension ? `safari-extension:${filename}` : `safari-web-extension:${filename}`,
146+
]
147+
: [func, filename];
148+
};

0 commit comments

Comments
 (0)