Skip to content

Commit 1dda724

Browse files
timfishHazAT
authored andcommitted
Fix: Correctly parse stack with braces in path (getsentry#1948)
* Fork stack-trace * Remove stack-trace dependency * Oops, fix parser tests * Add attribution to tests and disable prefer-template for file
1 parent c9b15ea commit 1dda724

File tree

6 files changed

+276
-17
lines changed

6 files changed

+276
-17
lines changed

packages/node/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,9 @@
2020
"@sentry/hub": "5.0.0-beta0",
2121
"@sentry/types": "5.0.0-beta0",
2222
"@sentry/utils": "5.0.0-beta0",
23-
"@types/stack-trace": "0.0.29",
2423
"cookie": "0.3.1",
2524
"https-proxy-agent": "2.2.1",
2625
"lru_map": "0.3.3",
27-
"stack-trace": "0.0.10",
2826
"tslib": "^1.9.3"
2927
},
3028
"devDependencies": {

packages/node/src/parsers.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import { snipLine } from '@sentry/utils/string';
44
import { SyncPromise } from '@sentry/utils/syncpromise';
55
import { readFile } from 'fs';
66
import { LRUMap } from 'lru_map';
7-
import * as stacktrace from 'stack-trace';
87

98
import { NodeOptions } from './backend';
9+
import * as stacktrace from './stack-trace';
1010

1111
// tslint:disable-next-line:no-unsafe-any
1212
const DEFAULT_LINES_OF_CONTEXT: number = 7;
@@ -23,7 +23,7 @@ export function resetFileContentCache(): void {
2323
/** JSDoc */
2424
function getFunction(frame: stacktrace.StackFrame): string {
2525
try {
26-
return frame.getFunctionName() || `${frame.getTypeName()}.${frame.getMethodName() || '<anonymous>'}`;
26+
return frame.functionName || `${frame.typeName}.${frame.methodName || '<anonymous>'}`;
2727
} catch (e) {
2828
// This seems to happen sometimes when using 'use strict',
2929
// stemming from `getTypeName`.
@@ -142,14 +142,14 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions
142142

143143
const frames: StackFrame[] = stack.map(frame => {
144144
const parsedFrame: StackFrame = {
145-
colno: frame.getColumnNumber(),
146-
filename: frame.getFileName() || '',
145+
colno: frame.columnNumber,
146+
filename: frame.fileName || '',
147147
function: getFunction(frame),
148-
lineno: frame.getLineNumber(),
148+
lineno: frame.lineNumber,
149149
};
150150

151151
const isInternal =
152-
frame.isNative() ||
152+
frame.native ||
153153
(parsedFrame.filename &&
154154
!parsedFrame.filename.startsWith('/') &&
155155
!parsedFrame.filename.startsWith('.') &&

packages/node/src/stack-trace.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* stack-trace - Parses node.js stack traces
3+
*
4+
* This was originally forked to fix this issue:
5+
* https://github.com/felixge/node-stack-trace/issues/31
6+
*
7+
* Mar 19,2019 - #4fd379e
8+
*
9+
* https://github.com/felixge/node-stack-trace/
10+
* @license MIT
11+
*/
12+
13+
/** Decoded StackFrame */
14+
export interface StackFrame {
15+
fileName: string;
16+
lineNumber: number;
17+
functionName: string;
18+
typeName: string;
19+
methodName: string;
20+
native: boolean;
21+
columnNumber: number;
22+
}
23+
24+
/** Extracts StackFrames fron the Error */
25+
export function parse(err: Error): StackFrame[] {
26+
if (!err.stack) {
27+
return [];
28+
}
29+
30+
const lines = err.stack.split('\n').slice(1);
31+
32+
return lines
33+
.map(line => {
34+
if (line.match(/^\s*[-]{4,}$/)) {
35+
return {
36+
columnNumber: null,
37+
fileName: line,
38+
functionName: null,
39+
lineNumber: null,
40+
methodName: null,
41+
native: null,
42+
typeName: null,
43+
};
44+
}
45+
46+
const lineMatch = line.match(/at (?:(.+?)\s+\()?(?:(.+?):(\d+)(?::(\d+))?|([^)]+))\)?/);
47+
if (!lineMatch) {
48+
return undefined;
49+
}
50+
51+
let object = null;
52+
let method = null;
53+
let functionName = null;
54+
let typeName = null;
55+
let methodName = null;
56+
const isNative = lineMatch[5] === 'native';
57+
58+
if (lineMatch[1]) {
59+
functionName = lineMatch[1];
60+
let methodStart = functionName.lastIndexOf('.');
61+
if (functionName[methodStart - 1] === '.') {
62+
methodStart--;
63+
}
64+
if (methodStart > 0) {
65+
object = functionName.substr(0, methodStart);
66+
method = functionName.substr(methodStart + 1);
67+
const objectEnd = object.indexOf('.Module');
68+
if (objectEnd > 0) {
69+
functionName = functionName.substr(objectEnd + 1);
70+
object = object.substr(0, objectEnd);
71+
}
72+
}
73+
typeName = null;
74+
}
75+
76+
if (method) {
77+
typeName = object;
78+
methodName = method;
79+
}
80+
81+
if (method === '<anonymous>') {
82+
methodName = null;
83+
functionName = null;
84+
}
85+
86+
const properties = {
87+
columnNumber: parseInt(lineMatch[4], 10) || null,
88+
fileName: lineMatch[2] || null,
89+
functionName,
90+
lineNumber: parseInt(lineMatch[3], 10) || null,
91+
methodName,
92+
native: isNative,
93+
typeName,
94+
};
95+
96+
return properties;
97+
})
98+
.filter(callSite => !!callSite) as StackFrame[];
99+
}

packages/node/test/parsers.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as fs from 'fs';
2-
import * as stacktrace from 'stack-trace';
32

43
import * as Parsers from '../src/parsers';
4+
import * as stacktrace from '../src/stack-trace';
55

66
import { getError } from './helper/error';
77

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
/**
2+
* stack-trace - Parses node.js stack traces
3+
*
4+
* These tests were originally forked to fix this issue:
5+
* https://github.com/felixge/node-stack-trace/issues/31
6+
*
7+
* Mar 19,2019 - #4fd379e
8+
*
9+
* https://github.com/felixge/node-stack-trace/
10+
* @license MIT
11+
*/
12+
13+
import * as stacktrace from '../src/stack-trace';
14+
15+
// tslint:disable:typedef
16+
// tslint:disable:prefer-template
17+
18+
function testBasic() {
19+
return new Error('something went wrong');
20+
}
21+
22+
function testWrapper() {
23+
return testBasic();
24+
}
25+
26+
describe('stack-trace.ts', () => {
27+
test('testObjectInMethodName', () => {
28+
const err: { [key: string]: any } = {};
29+
err.stack =
30+
'Error: Foo\n' +
31+
' at [object Object].global.every [as _onTimeout] (/Users/hoitz/develop/test.coffee:36:3)\n' +
32+
' at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)\n';
33+
34+
const trace = stacktrace.parse(err as Error);
35+
36+
expect(trace[0].fileName).toEqual('/Users/hoitz/develop/test.coffee');
37+
expect(trace[1].fileName).toEqual('timers.js');
38+
});
39+
40+
test('testBasic', () => {
41+
const trace = stacktrace.parse(testBasic());
42+
43+
expect(trace[0].fileName).toEqual(__filename);
44+
expect(trace[0].functionName).toEqual('testBasic');
45+
});
46+
47+
test('testWrapper', () => {
48+
const trace = stacktrace.parse(testWrapper());
49+
50+
expect(trace[0].functionName).toEqual('testBasic');
51+
expect(trace[1].functionName).toEqual('testWrapper');
52+
});
53+
54+
test('testNoStack', () => {
55+
const err = { stack: undefined };
56+
const trace = stacktrace.parse(err as Error);
57+
58+
expect(trace).toEqual([]);
59+
});
60+
61+
test('testCorruptStack', () => {
62+
const err: { [key: string]: any } = {};
63+
err.stack =
64+
'AssertionError: true == false\n' +
65+
' fuck' +
66+
' at Test.run (/Users/felix/code/node-fast-or-slow/lib/test.js:45:10)\n' +
67+
'oh no' +
68+
' at TestCase.run (/Users/felix/code/node-fast-or-slow/lib/test_case.js:61:8)\n';
69+
70+
const trace = stacktrace.parse(err as Error);
71+
72+
expect(trace.length).toEqual(2);
73+
});
74+
75+
test('testTraceWitoutColumnNumbers', () => {
76+
const err: { [key: string]: any } = {};
77+
err.stack =
78+
'AssertionError: true == false\n' +
79+
' at Test.fn (/Users/felix/code/node-fast-or-slow/test/fast/example/test-example.js:6)\n' +
80+
' at Test.run (/Users/felix/code/node-fast-or-slow/lib/test.js:45)';
81+
82+
const trace = stacktrace.parse(err as Error);
83+
84+
expect(trace[0].fileName).toEqual('/Users/felix/code/node-fast-or-slow/test/fast/example/test-example.js');
85+
expect(trace[0].lineNumber).toEqual(6);
86+
expect(trace[0].columnNumber).toEqual(null);
87+
});
88+
89+
test('testStackWithNativeCall', () => {
90+
const err: { [key: string]: any } = {};
91+
err.stack =
92+
'AssertionError: true == false\n' +
93+
' at Test.fn (/Users/felix/code/node-fast-or-slow/test/fast/example/test-example.js:6:10)\n' +
94+
' at Test.run (/Users/felix/code/node-fast-or-slow/lib/test.js:45:10)\n' +
95+
' at TestCase.runNext (/Users/felix/code/node-fast-or-slow/lib/test_case.js:73:8)\n' +
96+
' at TestCase.run (/Users/felix/code/node-fast-or-slow/lib/test_case.js:61:8)\n' +
97+
' at Array.0 (native)\n' +
98+
' at EventEmitter._tickCallback (node.js:126:26)';
99+
100+
const trace = stacktrace.parse(err as Error);
101+
const nativeCallSite = trace[4];
102+
103+
expect(nativeCallSite.fileName).toEqual(null);
104+
expect(nativeCallSite.functionName).toEqual('Array.0');
105+
expect(nativeCallSite.typeName).toEqual('Array');
106+
expect(nativeCallSite.methodName).toEqual('0');
107+
expect(nativeCallSite.lineNumber).toEqual(null);
108+
expect(nativeCallSite.columnNumber).toEqual(null);
109+
expect(nativeCallSite.native).toEqual(true);
110+
});
111+
112+
test('testStackWithFileOnly', () => {
113+
const err: { [key: string]: any } = {};
114+
err.stack = 'AssertionError: true == false\n' + ' at /Users/felix/code/node-fast-or-slow/lib/test_case.js:80:10';
115+
116+
const trace = stacktrace.parse(err as Error);
117+
const callSite = trace[0];
118+
119+
expect(callSite.fileName).toEqual('/Users/felix/code/node-fast-or-slow/lib/test_case.js');
120+
expect(callSite.functionName).toEqual(null);
121+
expect(callSite.typeName).toEqual(null);
122+
expect(callSite.methodName).toEqual(null);
123+
expect(callSite.lineNumber).toEqual(80);
124+
expect(callSite.columnNumber).toEqual(10);
125+
expect(callSite.native).toEqual(false);
126+
});
127+
128+
test('testStackWithMultilineMessage', () => {
129+
const err: { [key: string]: any } = {};
130+
err.stack =
131+
'AssertionError: true == false\nAnd some more shit\n' +
132+
' at /Users/felix/code/node-fast-or-slow/lib/test_case.js:80:10';
133+
134+
const trace = stacktrace.parse(err as Error);
135+
const callSite = trace[0];
136+
137+
expect(callSite.fileName).toEqual('/Users/felix/code/node-fast-or-slow/lib/test_case.js');
138+
});
139+
140+
test('testStackWithAnonymousFunctionCall', () => {
141+
const err: { [key: string]: any } = {};
142+
err.stack =
143+
'AssertionError: expected [] to be arguments\n' +
144+
' at Assertion.prop.(anonymous function) (/Users/den/Projects/should.js/lib/should.js:60:14)\n';
145+
146+
const trace = stacktrace.parse(err as Error);
147+
const callSite0 = trace[0];
148+
149+
expect(callSite0.fileName).toEqual('/Users/den/Projects/should.js/lib/should.js');
150+
expect(callSite0.functionName).toEqual('Assertion.prop.(anonymous function)');
151+
expect(callSite0.typeName).toEqual('Assertion.prop');
152+
expect(callSite0.methodName).toEqual('(anonymous function)');
153+
expect(callSite0.lineNumber).toEqual(60);
154+
expect(callSite0.columnNumber).toEqual(14);
155+
expect(callSite0.native).toEqual(false);
156+
});
157+
158+
test('testTraceBracesInPath', () => {
159+
const err: { [key: string]: any } = {};
160+
err.stack =
161+
'AssertionError: true == false\n' +
162+
' at Test.run (/Users/felix (something)/code/node-fast-or-slow/lib/test.js:45:10)\n' +
163+
' at TestCase.run (/Users/felix (something)/code/node-fast-or-slow/lib/test_case.js:61:8)\n';
164+
165+
const trace = stacktrace.parse(err as Error);
166+
167+
expect(trace.length).toEqual(2);
168+
expect(trace[0].fileName).toEqual('/Users/felix (something)/code/node-fast-or-slow/lib/test.js');
169+
});
170+
});

yarn.lock

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,10 +1325,6 @@
13251325
resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-7.0.10.tgz#1f921f0c347b19f754e61dbc671c088df73fe1ff"
13261326
integrity sha512-4w7SvsiUOtd4mUfund9QROPSJ5At/GQskDpqd87pJIRI6ULWSJqHI3GIZE337wQuN3aznroJGr94+o8fwvL37Q==
13271327

1328-
"@types/stack-trace@0.0.29":
1329-
version "0.0.29"
1330-
resolved "https://registry.yarnpkg.com/@types/stack-trace/-/stack-trace-0.0.29.tgz#eb7a7c60098edb35630ed900742a5ecb20cfcb4d"
1331-
13321328
"@types/stack-utils@^1.0.1":
13331329
version "1.0.1"
13341330
resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-1.0.1.tgz#0a851d3bd96498fa25c33ab7278ed3bd65f06c3e"
@@ -9803,10 +9799,6 @@ ssri@^6.0.0, ssri@^6.0.1:
98039799
dependencies:
98049800
figgy-pudding "^3.5.1"
98059801

9806-
stack-trace@0.0.10:
9807-
version "0.0.10"
9808-
resolved "https://registry.yarnpkg.com/stack-trace/-/stack-trace-0.0.10.tgz#547c70b347e8d32b4e108ea1a2a159e5fdde19c0"
9809-
98109802
stack-utils@^1.0.1:
98119803
version "1.0.2"
98129804
resolved "https://registry.yarnpkg.com/stack-utils/-/stack-utils-1.0.2.tgz#33eba3897788558bebfc2db059dc158ec36cebb8"

0 commit comments

Comments
 (0)