Skip to content

Commit 4eadcaf

Browse files
timfishtimfish
authored
ref(node): Fix async stack parsing (#4721)
This fixes a number of issues with node stack parsing: - Regex did not cater for `async` stack frames (introduced node v12) - `file://` was not being stripped from paths - Some anonymous functions were ending up as `undefined.<anonymous>`
1 parent a0cb3bb commit 4eadcaf

File tree

2 files changed

+111
-21
lines changed

2 files changed

+111
-21
lines changed

‎packages/node/src/stack-parser.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ function getModule(filename: string | undefined): string | undefined {
3636
}
3737

3838
const FILENAME_MATCH = /^\s*[-]{4,}$/;
39-
const FULL_MATCH = /at (?:(.+?)\s+\()?(?:(.+?):(\d+)(?::(\d+))?|([^)]+))\)?/;
39+
const FULL_MATCH = /at (?:async )?(?:(.+?)\s+\()?(?:(.+?):(\d+)(?::(\d+))?|([^)]+))\)?/;
4040

41+
// eslint-disable-next-line complexity
4142
const node: StackLineParserFn = (line: string) => {
4243
if (line.match(FILENAME_MATCH)) {
4344
return {
@@ -87,7 +88,12 @@ const node: StackLineParserFn = (line: string) => {
8788
functionName = undefined;
8889
}
8990

90-
const filename = lineMatch[2];
91+
if (functionName === undefined) {
92+
methodName = methodName || '<anonymous>';
93+
functionName = typeName ? `${typeName}.${methodName}` : methodName;
94+
}
95+
96+
const filename = lineMatch[2]?.startsWith('file://') ? lineMatch[2].substr(7) : lineMatch[2];
9197
const isNative = lineMatch[5] === 'native';
9298
const isInternal =
9399
isNative || (filename && !filename.startsWith('/') && !filename.startsWith('.') && filename.indexOf(':\\') !== 1);
@@ -100,7 +106,7 @@ const node: StackLineParserFn = (line: string) => {
100106
return {
101107
filename,
102108
module: getModule(filename),
103-
function: functionName || `${typeName}.${methodName || '<anonymous>'}`,
109+
function: functionName,
104110
lineno: parseInt(lineMatch[3], 10) || undefined,
105111
colno: parseInt(lineMatch[4], 10) || undefined,
106112
in_app,

‎packages/node/test/stacktrace.test.ts‎

Lines changed: 102 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ describe('Stack parsing', () => {
5353
});
5454

5555
test('parses object in fn name', () => {
56-
const err: { [key: string]: any } = {};
56+
const err = new Error();
5757
err.stack =
5858
'Error: Foo\n' +
5959
' at [object Object].global.every [as _onTimeout] (/Users/hoitz/develop/test.coffee:36:3)\n' +
6060
' at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)\n';
6161

62-
const frames = parseStackFrames(err as Error);
62+
const frames = parseStackFrames(err);
6363

6464
expect(frames).toEqual([
6565
{
@@ -89,15 +89,15 @@ describe('Stack parsing', () => {
8989
});
9090

9191
test('parses corrupt stack', () => {
92-
const err: { [key: string]: any } = {};
92+
const err = new Error();
9393
err.stack =
9494
'AssertionError: true == false\n' +
9595
' fuck' +
9696
' at Test.run (/Users/felix/code/node-fast-or-slow/lib/test.js:45:10)\n' +
9797
'oh no' +
9898
' at TestCase.run (/Users/felix/code/node-fast-or-slow/lib/test_case.js:61:8)\n';
9999

100-
const frames = parseStackFrames(err as Error);
100+
const frames = parseStackFrames(err);
101101

102102
expect(frames).toEqual([
103103
{
@@ -120,13 +120,13 @@ describe('Stack parsing', () => {
120120
});
121121

122122
test('parses with missing column numbers', () => {
123-
const err: { [key: string]: any } = {};
123+
const err = new Error();
124124
err.stack =
125125
'AssertionError: true == false\n' +
126126
' at Test.fn (/Users/felix/code/node-fast-or-slow/test/fast/example/test-example.js:6)\n' +
127127
' at Test.run (/Users/felix/code/node-fast-or-slow/lib/test.js:45)';
128128

129-
const frames = parseStackFrames(err as Error);
129+
const frames = parseStackFrames(err);
130130

131131
expect(frames).toEqual([
132132
{
@@ -147,7 +147,7 @@ describe('Stack parsing', () => {
147147
});
148148

149149
test('parses with native methods', () => {
150-
const err: { [key: string]: any } = {};
150+
const err = new Error();
151151
err.stack =
152152
'AssertionError: true == false\n' +
153153
' at Test.fn (/Users/felix/code/node-fast-or-slow/test/fast/example/test-example.js:6:10)\n' +
@@ -157,7 +157,7 @@ describe('Stack parsing', () => {
157157
' at Array.0 (native)\n' +
158158
' at EventEmitter._tickCallback (node.js:126:26)';
159159

160-
const frames = parseStackFrames(err as Error);
160+
const frames = parseStackFrames(err);
161161

162162
expect(frames).toEqual([
163163
{
@@ -209,16 +209,16 @@ describe('Stack parsing', () => {
209209
});
210210

211211
test('parses with file only', () => {
212-
const err: { [key: string]: any } = {};
212+
const err = new Error();
213213
err.stack = 'AssertionError: true == false\n' + ' at /Users/felix/code/node-fast-or-slow/lib/test_case.js:80:10';
214214

215-
const frames = parseStackFrames(err as Error);
215+
const frames = parseStackFrames(err);
216216

217217
expect(frames).toEqual([
218218
{
219219
filename: '/Users/felix/code/node-fast-or-slow/lib/test_case.js',
220220
module: 'test_case',
221-
function: 'undefined.<anonymous>',
221+
function: '<anonymous>',
222222
lineno: 80,
223223
colno: 10,
224224
in_app: true,
@@ -227,18 +227,18 @@ describe('Stack parsing', () => {
227227
});
228228

229229
test('parses with multi line message', () => {
230-
const err: { [key: string]: any } = {};
230+
const err = new Error();
231231
err.stack =
232232
'AssertionError: true == false\nAnd some more shit\n' +
233233
' at /Users/felix/code/node-fast-or-slow/lib/test_case.js:80:10';
234234

235-
const frames = parseStackFrames(err as Error);
235+
const frames = parseStackFrames(err);
236236

237237
expect(frames).toEqual([
238238
{
239239
filename: '/Users/felix/code/node-fast-or-slow/lib/test_case.js',
240240
module: 'test_case',
241-
function: 'undefined.<anonymous>',
241+
function: '<anonymous>',
242242
lineno: 80,
243243
colno: 10,
244244
in_app: true,
@@ -247,12 +247,12 @@ describe('Stack parsing', () => {
247247
});
248248

249249
test('parses with anonymous fn call', () => {
250-
const err: { [key: string]: any } = {};
250+
const err = new Error();
251251
err.stack =
252252
'AssertionError: expected [] to be arguments\n' +
253253
' at Assertion.prop.(anonymous function) (/Users/den/Projects/should.js/lib/should.js:60:14)\n';
254254

255-
const frames = parseStackFrames(err as Error);
255+
const frames = parseStackFrames(err);
256256

257257
expect(frames).toEqual([
258258
{
@@ -267,13 +267,13 @@ describe('Stack parsing', () => {
267267
});
268268

269269
test('parses with braces in paths', () => {
270-
const err: { [key: string]: any } = {};
270+
const err = new Error();
271271
err.stack =
272272
'AssertionError: true == false\n' +
273273
' at Test.run (/Users/felix (something)/code/node-fast-or-slow/lib/test.js:45:10)\n' +
274274
' at TestCase.run (/Users/felix (something)/code/node-fast-or-slow/lib/test_case.js:61:8)\n';
275275

276-
const frames = parseStackFrames(err as Error);
276+
const frames = parseStackFrames(err);
277277

278278
expect(frames).toEqual([
279279
{
@@ -294,4 +294,88 @@ describe('Stack parsing', () => {
294294
},
295295
]);
296296
});
297+
298+
test('parses with async frames', () => {
299+
// https://github.com/getsentry/sentry-javascript/issues/4692#issuecomment-1063835795
300+
const err = new Error();
301+
err.stack =
302+
'Error: Client request error\n' +
303+
' at Object.httpRequestError (file:///code/node_modules/@waroncancer/gaia/lib/error/error-factory.js:17:73)\n' +
304+
' at Object.run (file:///code/node_modules/@waroncancer/gaia/lib/http-client/http-client.js:81:36)\n' +
305+
' at processTicksAndRejections (node:internal/process/task_queues:96:5)\n' +
306+
' at async Object.send (file:///code/lib/post-created/send-post-created-notification-module.js:17:27)\n' +
307+
' at async each (file:///code/lib/process-post-events-module.js:14:21)\n' +
308+
' at async Runner.processEachMessage (/code/node_modules/kafkajs/src/consumer/runner.js:151:9)\n' +
309+
' at async onBatch (/code/node_modules/kafkajs/src/consumer/runner.js:326:9)\n' +
310+
' at async /code/node_modules/kafkajs/src/consumer/runner.js:376:15\n';
311+
312+
const frames = parseStackFrames(err);
313+
314+
expect(frames).toEqual([
315+
{
316+
filename: '/code/node_modules/kafkajs/src/consumer/runner.js',
317+
module: 'kafkajs.src.consumer:runner',
318+
function: '<anonymous>',
319+
lineno: 376,
320+
colno: 15,
321+
in_app: false,
322+
},
323+
{
324+
filename: '/code/node_modules/kafkajs/src/consumer/runner.js',
325+
module: 'kafkajs.src.consumer:runner',
326+
function: 'onBatch',
327+
lineno: 326,
328+
colno: 9,
329+
in_app: false,
330+
},
331+
{
332+
filename: '/code/node_modules/kafkajs/src/consumer/runner.js',
333+
module: 'kafkajs.src.consumer:runner',
334+
function: 'Runner.processEachMessage',
335+
lineno: 151,
336+
colno: 9,
337+
in_app: false,
338+
},
339+
{
340+
filename: '/code/lib/process-post-events-module.js',
341+
module: 'process-post-events-module',
342+
function: 'each',
343+
lineno: 14,
344+
colno: 21,
345+
in_app: true,
346+
},
347+
{
348+
filename: '/code/lib/post-created/send-post-created-notification-module.js',
349+
module: 'send-post-created-notification-module',
350+
function: 'Object.send',
351+
lineno: 17,
352+
colno: 27,
353+
in_app: true,
354+
},
355+
{
356+
filename: 'node:internal/process/task_queues',
357+
module: 'task_queues',
358+
function: 'processTicksAndRejections',
359+
lineno: 96,
360+
colno: 5,
361+
in_app: false,
362+
},
363+
{
364+
filename: '/code/node_modules/@waroncancer/gaia/lib/http-client/http-client.js',
365+
module: '@waroncancer.gaia.lib.http-client:http-client',
366+
function: 'Object.run',
367+
lineno: 81,
368+
colno: 36,
369+
in_app: false,
370+
},
371+
{
372+
filename: '/code/node_modules/@waroncancer/gaia/lib/error/error-factory.js',
373+
module: '@waroncancer.gaia.lib.error:error-factory',
374+
function: 'Object.httpRequestError',
375+
lineno: 17,
376+
colno: 73,
377+
in_app: false,
378+
},
379+
]);
380+
});
297381
});

0 commit comments

Comments
 (0)