8000 fix(browser): Parse frames-only safari(-web)-extension stack (#3929) · ssnielsen/sentry-javascript@45508c0 · GitHub
[go: up one dir, main page]
Skip to content

Commit 45508c0

Browse files
kamilogorekkamilogorek
authored
fix(browser): Parse frames-only safari(-web)-extension stack (getsentry#3929)
* fix(browser): Parse frames-only safari(-web)-extension stack * Simplify function and add appropriate comment
1 parent 88b96f6 commit 45508c0

File tree

2 files changed

+160
-57
lines changed

2 files changed

+160
-57
lines changed

‎packages/browser/src/tracekit.ts

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* largely modified and is now maintained as part of Sentry JS SDK.
44
*/
55

6-
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
6+
/* eslint-disable @typescript-eslint/no-unsafe-member-access, max-lines */
77

88
/**
99
* An object representing a single stack frame.
@@ -124,16 +124,10 @@ function computeStackTraceFromStackProp(ex: any): StackTrace | null {
124124
// Arpad: Working with the regexp above is super painful. it is quite a hack, but just stripping the `address at `
125125
// prefix here seems like the quickest solution for now.
126126
let url = parts[2] && parts[2].indexOf('address at ') === 0 ? parts[2].substr('address at '.length) : parts[2];
127-
128127
// Kamil: One more hack won't hurt us right? Understanding and adding more rules on top of these regexps right now
129128
// would be way too time consuming. (TODO: Rewrite whole RegExp to be more readable)
130129
let func = parts[1] || UNKNOWN_FUNCTION;
131-
const isSafariExtension = func.indexOf('safari-extension') !== -1;
132-
const isSafariWebExtension = func.indexOf('safari-web-extension') !== -1;
133-
if (isSafariExtension || isSafariWebExtension) {
134-
func = func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION;
135-
url = isSafariExtension ? `safari-extension:${url}` : `safari-web-extension:${url}`;
136-
}
130+
[func, url] = extractSafariExtensionDetails(func, url);
137131

138132
element = {
139133
url,
@@ -165,9 +159,14 @@ function computeStackTraceFromStackProp(ex: any): StackTrace | null {
165159
// NOTE: this hack doesn't work if top-most frame is eval
166160
stack[0].column = (ex.columnNumber as number) + 1;
167161
}
162+
163+
let url = parts[3];
164+
let func = parts[1] || UNKNOWN_FUNCTION;
165+
[func, url] = extractSafariExtensionDetails(func, url);
166+
168167
element = {
169-
url: parts[3],
170-
func: parts[1] || UNKNOWN_FUNCTION,
168+
url,
169+
func,
171170
args: parts[2] ? parts[2].split(',') : [],
172171
line: parts[4] ? +parts[4] : null,
173172
column: parts[5] ? +parts[5] : null,
@@ -249,6 +248,38 @@ function computeStackTraceFromStacktraceProp(ex: any): StackTrace | null {
249248
};
250249
}
251250

251+
/**
252+
* Safari web extensions, starting version unknown, can produce "frames-only" stacktraces.
253+
* What it means, is that instead of format like:
254+
*
255+
* Error: wat
256+
* at function@url:row:col
257+
* at function@url:row:col
258+
* at function@url:row:col
259+
*
260+
* it produces something like:
261+
*
262+
* function@url:row:col
263+
* function@url:row:col
264+
* function@url:row:col
265+
*
266+
* Because of that, it won't be captured by `chrome` RegExp and will fall into `Gecko` branch.
267+
* This function is extracted so that we can use it in both places without duplicating the logic.
268+
* Unfortunatelly "just" changing RegExp is too complicated now and making it pass all tests
269+
* and fix this case seems like an impossible, or at least way too time-consuming task.
270+
*/
271+
const extractSafariExtensionDetails = (func: string, url: string): [string, string] => {
272+
const isSafariExtension = func.indexOf('safari-extension') !== -1;
273+
const isSafariWebExtension = func.indexOf('safari-web-extension') !== -1;
274+
275+
return isSafariExtension || isSafariWebExtension
276+
? [
277+
func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION,
278+
isSafariExtension ? `safari-extension:${url}` : `safari-web-extension:${url}`,
279+
]
280+
: [func, url];
281+
};
282+
252283
/** Remove N number of frames from the stack */
253284< 9E88 /td>
function popFrames(stacktrace: StackTrace, popSize: number): StackTrace {
254285
try {

‎packages/browser/test/unit/tracekit/custom.test.ts

Lines changed: 119 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -24,58 +24,130 @@ describe('Tracekit - Custom Tests', () => {
2424
]);
2525
});
2626

27-
it('should parse exceptions for safari-extension', () => {
28-
const SAFARI_EXTENSION_EXCEPTION = {
29-
message: 'wat',
30-
name: 'Error',
31-
stack: `Error: wat
27+
describe('Safari extensions', () => {
28+
it('should parse exceptions for safari-extension', () => {
29+
const SAFARI_EXTENSION_EXCEPTION = {
30+
message: 'wat',
31+
name: 'Error',
32+
stack: `Error: wat
3233
at ClipperError@safari-extension:(//3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/commons.js:223036:10)
3334
at safari-extension:(//3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/topee-content.js:3313:26)`,
34-
};
35-
const stacktrace = computeStackTrace(SAFARI_EXTENSION_EXCEPTION);
36-
expect(stacktrace.stack).deep.equal([
37-
{
38-
url: 'safari-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/commons.js',
39-
func: 'ClipperError',
40-
args: [],
41-
line: 223036,
42-
column: 10,
43-
},
44-
{
45-
url: 'safari-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/topee-content.js',
46-
func: '?',
47-
args: [],
48-
line: 3313,
49-
column: 26,
50-
},
51-
]);
52-
});
35+
};
36+
const stacktrace = computeStackTrace(SAFARI_EXTENSION_EXCEPTION);
37+
expect(stacktrace.stack).deep.equal([
38+
{
39+
url: 'safari-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/commons.js',
40+
func: 'ClipperError',
41+
args: [],
42+
line: 223036,
43+
column: 10,
44+
},
45+
{
46+
url: 'safari-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/topee-content.js',
47+
func: '?',
48+
args: [],
49+
line: 3313,
50+
column: 26,
51+
},
52+
]);
53+
});
5354

54-
it('should parse exceptions for safari-web-extension', () => {
55-
const SAFARI_WEB_EXTENSION_EXCEPTION = {
56-
message: 'wat',
57-
name: 'Error',
58-
stack: `Error: wat
55+
it('should parse exceptions for safari-extension with frames-only stack', () => {
56+
const SAFARI_EXTENSION_EXCEPTION = {
57+
message: `undefined is not an object (evaluating 'e.groups.includes')`,
58+
name: `TypeError`,
59+
stack: `isClaimed@safari-extension://com.grammarly.safari.extension.ext2-W8F64X92K3/ee7759dd/Grammarly.js:2:929865
60+
safari-extension://com.grammarly.safari.extension.ext2-W8F64X92K3/ee7759dd/Grammarly.js:2:1588410
61+
promiseReactionJob@[native code]`,
62+
};
63+
const stacktrace = computeStackTrace(SAFARI_EXTENSION_EXCEPTION);
64+
65+
expect(stacktrace.stack).deep.equal([
66+
{
67+
url: 'safari-extension://com.grammarly.safari.extension.ext2-W8F64X92K3/ee7759dd/Grammarly.js',
68+
func: 'isClaimed',
69+
args: [],
70+
line: 2,
71+
column: 929865,
72+
},
73+
{
74+
url: 'safari-extension://com.grammarly.safari.extension.ext2-W8F64X92K3/ee7759dd/Grammarly.js',
75+
func: '?',
76+
args: [],
77+
line: 2,
78+
column: 1588410,
79+
},
80+
{
81+
url: '[native code]',
82+
func: 'promiseReactionJob',
83+
args: [],
84+
line: null,
85+
column: null,
86+
},
87+
]);
88+
});
89+
90+
it('should parse exceptions for safari-web-extension', () => {
91+
const SAFARI_WEB_EXTENSION_EXCEPTION = {
92+
message: 'wat',
93+
name: 'Error',
94+
stack: `Error: wat
5995
at ClipperError@safari-web-extension:(//3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/commons.js:223036:10)
6096
at safari-web-extension:(//3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/topee-content.js:3313:26)`,
61-
};
62-
const stacktrace = computeStackTrace(SAFARI_WEB_EXTENSION_EXCEPTION);
63-
expect(stacktrace.stack).deep.equal([
64-
{
65-
url: 'safari-web-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/commons.js',
66-
func: 'ClipperError',
67-
args: [],
68-
line: 223036,
69-
column: 10,
70-
},
71-
{
72-
url: 'safari-web-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/topee-content.js',
73-
func: '?',
74-
args: [],
75-
line: 3313,
76-
column: 26,
77-
},
78-
]);
97+
};
98+
const stacktrace = computeStackTrace(SAFARI_WEB_EXTENSION_EXCEPTION);
99+
expect(stacktrace.stack).deep.equal([
100+
{
101+
url: 'safari-web-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/commons.js',
102+
func: 'ClipperError',
103+
args: [],
104+
line: 223036,
105+
column: 10,
106+
},
107+
{
108+
url: 'safari-web-extension://3284871F-A480-4FFC-8BC4-3F362C752446/2665fee0/topee-content.js',
109+
func: '?',
110+
args: [],
111+
line: 3313,
112+
column: 26,
113+
},
114+
]);
115+
});
116+
117+
it('should parse exceptions for safari-web-extension with frames-only stack', () => {
118+
const SAFARI_EXTENSION_EXCEPTION = {
119+
message: `undefined is not an object (evaluating 'e.groups.includes')`,
120+
name: `TypeError`,
121+
stack: `p_@safari-web-extension://46434E60-F5BD-48A4-80C8-A422C5D16897/scripts/content-script.js:29:33314
122+
safari-web-extension://46434E60-F5BD-48A4-80C8-A422C5D16897/scripts/content-script.js:29:56027
123+
promiseReactionJob@[native code]`,
124+
};
125+
const stacktrace = computeStackTrace(SAFARI_EXTENSION_EXCEPTION);
126+
127+
expect(stacktrace.stack).deep.equal([
128+
{
129+
url: 'safari-web-extension://46434E60-F5BD-48A4-80C8-A422C5D16897/scripts/content-script.js',
130+
func: 'p_',
131+
args: [],
132+
line: 29,
133+
column: 33314,
134+
},
135+
{
136+
url: 'safari-web-extension://46434E60-F5BD-48A4-80C8-A422C5D16897/scripts/content-script.js',
137+
func: '?',
138+
args: [],
139+
line: 29,
140+
column: 56027,
141+
},
142+
{
143+
url: '[native code]',
144+
func: 'promiseReactionJob',
145+
args: [],
146+
line: null,
147+
column: null,
148+
},
149+
]);
150+
});
79151
});
80152

81153
it('should parse exceptions for react-native-v8', () => {

0 commit comments

Comments
 (0)
0