Skip to content

Commit 0f2772a

Browse files
committed
fix: address PR #66 review, fix #29, verify #13 and #25
- Fix issue #29: parse() now captures source location from SyntaxError stacks where V8 puts the file:line(:col) as the first line instead of an error message. This is returned as the first CallSite frame. - Source location detection uses robust guards: * Excludes lines with ': ' (colon+space) which indicates error messages * Excludes URL schemes (http://, https://, etc.) * Regex is safe from ReDoS (non-greedy with anchored terminus) - Refactored parse() internals from map+filter to forEach+push: * Identical output behavior (verified with tests) * Slightly more memory efficient (1 array vs 2 intermediate arrays) * Fully compatible with Node 20+ - Keep engines.node at >=20.0.0 - Fix README: 'All other methods' -> 'The other getter-style methods' (boolean predicates return false, not null) - Add full boundary-frame contract assertions to long-stack-trace test - Remove dead 'exceptions' parameter from compare() in parse-test - Add regression test for issue #13 ([object Object] in function name) -- confirmed already working with current regex - Add verification test for issue #25 (async/await) -- confirmed resolved on modern Node via V8 async stack traces - Comprehensive test coverage for #29: SyntaxError with/without column, Windows paths, non-regression for Error/TypeError/RangeError, custom error types, URL schemes, adversarial ReDoS inputs, forEach output equivalence, and memory/shared-state safety Closes #13 Closes #25 Closes #29
1 parent e19d06c commit 0f2772a

5 files changed

Lines changed: 312 additions & 19 deletions

File tree

‎Readme.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ certain properties can be retrieved with it as noted in the API docs below.
4141
When parsing an `err.stack` that has crossed the event loop boundary, a
4242
`CallSite` object is created whose `getFileName()` returns the full dashed
4343
separator line from the stack, including any leading whitespace such as
44-
indentation. All other methods of the event loop boundary call site return
45-
`null`.
44+
indentation. The other getter-style methods of the event loop boundary call site
45+
return `null`.
4646

4747
Historically this behavior was often observed together with
4848
[long-stack-traces](https://github.com/tlrobinson/long-stack-traces), but that package is unmaintained. This module does not depend on it and still supports parsing dashed event-loop boundary markers when

‎__tests__/get-test.js‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it } from 'node:test';
22
import assert from 'node:assert/strict';
3-
import { get } from "../index.js";
3+
import { get, parse } from "../index.js";
44

55
describe("get", () => {
66
it("basic", () => {
@@ -48,4 +48,24 @@ describe("get", () => {
4848
})();
4949
})();
5050
});
51+
52+
// Verification for https://github.com/felixge/node-stack-trace/issues/25
53+
// V8 async stack traces (enabled by default since Node 12) ensure async/await
54+
// callers appear in the captured stack.
55+
it("async/await stack traces include caller frames", async () => {
56+
async function innerAsync() {
57+
return new Error('async trace');
58+
}
59+
async function outerAsync() {
60+
return await innerAsync();
61+
}
62+
63+
const err = await outerAsync();
64+
const trace = parse(err);
65+
66+
const hasInner = trace.some(t => t.getFunctionName() === 'innerAsync');
67+
const hasOuter = trace.some(t => t.getFunctionName() === 'outerAsync');
68+
assert.strictEqual(hasInner, true, 'should include innerAsync frame');
69+
assert.strictEqual(hasOuter, true, 'should include outerAsync frame');
70+
});
5171
});

‎__tests__/long-stack-trace-test.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,12 @@ describe("long stack trace", () => {
3232

3333
assert.notStrictEqual(boundary, undefined);
3434
assert.match(boundary.getFileName(), /-----/);
35+
assert.strictEqual(boundary.getFunctionName(), null);
36+
assert.strictEqual(boundary.getMethodName(), null);
37+
assert.strictEqual(boundary.getTypeName(), null);
38+
assert.strictEqual(boundary.getLineNumber(), null);
39+
assert.strictEqual(boundary.getColumnNumber(), null);
40+
assert.strictEqual(boundary.getEvalOrigin(), null);
41+
assert.strictEqual(boundary.isNative(), null);
3542
});
3643
});

‎__tests__/parse-test.js‎

Lines changed: 249 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,23 @@ describe("parse", () => {
1515
assert.strictEqual(trace[1].getFileName(), "timers.js");
1616
});
1717

18+
// Regression test for https://github.com/felixge/node-stack-trace/issues/13
19+
it("[object Object] as type name in function", () => {
20+
const err = {};
21+
err.stack =
22+
'Error: Could not do something\n' +
23+
' at [object Object].foo.bar (foo.js:1:2)\n';
24+
25+
const trace = parse(err);
26+
assert.strictEqual(trace[0].getFileName(), 'foo.js');
27+
assert.strictEqual(trace[0].getFunctionName(), '[object Object].foo.bar');
28+
assert.strictEqual(trace[0].getTypeName(), '[object Object].foo');
29+
assert.strictEqual(trace[0].getMethodName(), 'bar');
30+
assert.strictEqual(trace[0].getLineNumber(), 1);
31+
assert.strictEqual(trace[0].getColumnNumber(), 2);
32+
assert.strictEqual(trace[0].isNative(), false);
33+
});
34+
1835
it("basic", () => {
1936
(function testBasic() {
2037
const err = new Error('something went wrong');
@@ -117,14 +134,10 @@ describe("parse", () => {
117134
userFrames++;
118135
}
119136

120-
function compare(method, exceptions) {
121-
let realValue = real[method]();
137+
function compare(method) {
138+
const realValue = real[method]();
122139
const parsedValue = parsed[method]();
123140

124-
if (exceptions && typeof exceptions[i] != 'undefined') {
125-
realValue = exceptions[i];
126-
}
127-
128141
assert.strictEqual(realValue, parsedValue);
129142
}
130143

@@ -212,4 +225,234 @@ describe("parse", () => {
212225
assert.strictEqual(callSite0.getColumnNumber(), 14);
213226
assert.strictEqual(callSite0.isNative(), false);
214227
});
228+
229+
// Regression tests for https://github.com/felixge/node-stack-trace/issues/29
230+
// SyntaxError stacks include the source location as the first line instead of
231+
// the error message. parse() should capture it as the first frame.
232+
it("SyntaxError stack includes source location as first frame", () => {
233+
const err = {};
234+
err.stack =
235+
'/path/to/source/file.js:22\n' +
236+
'(*$&@#(*!route.validate();\n' +
237+
' ^\n' +
238+
'\n' +
239+
'SyntaxError: Unexpected token *\n' +
240+
' at createScript (vm.js:80:10)\n' +
241+
' at Object.runInThisContext (vm.js:139:10)';
242+
243+
const trace = parse(err);
244+
245+
// First frame should be the source location
246+
assert.strictEqual(trace[0].getFileName(), '/path/to/source/file.js');
247+
assert.strictEqual(trace[0].getLineNumber(), 22);
248+
assert.strictEqual(trace[0].getColumnNumber(), null);
249+
assert.strictEqual(trace[0].getFunctionName(), null);
250+
assert.strictEqual(trace[0].getTypeName(), null);
251+
assert.strictEqual(trace[0].getMethodName(), null);
252+
assert.strictEqual(trace[0].isNative(), false);
253+
254+
// Remaining frames should still be parsed normally
255+
assert.strictEqual(trace[1].getFunctionName(), 'createScript');
256+
assert.strictEqual(trace[1].getFileName(), 'vm.js');
257+
assert.strictEqual(trace[1].getLineNumber(), 80);
258+
assert.strictEqual(trace[2].getFunctionName(), 'Object.runInThisContext');
259+
assert.strictEqual(trace[2].getFileName(), 'vm.js');
260+
assert.strictEqual(trace[2].getLineNumber(), 139);
261+
});
262+
263+
it("SyntaxError stack with column number in source location", () => {
264+
const err = {};
265+
err.stack =
266+
'/path/to/source/file.js:22:5\n' +
267+
'unexpected code here\n' +
268+
' ^\n' +
269+
'\n' +
270+
'SyntaxError: Unexpected identifier\n' +
271+
' at Module._compile (internal/modules/cjs/loader.js:723:23)';
272+
273+
const trace = parse(err);
274+
275+
assert.strictEqual(trace[0].getFileName(), '/path/to/source/file.js');
276+
assert.strictEqual(trace[0].getLineNumber(), 22);
277+
assert.strictEqual(trace[0].getColumnNumber(), 5);
278+
assert.strictEqual(trace[0].getFunctionName(), null);
279+
assert.strictEqual(trace[0].isNative(), false);
280+
281+
assert.strictEqual(trace[1].getFunctionName(), 'Module._compile');
282+
assert.strictEqual(trace[1].getLineNumber(), 723);
283+
});
284+
285+
it("normal Error stack is not affected by source location detection", () => {
286+
const err = {};
287+
err.stack =
288+
'Error: something went wrong\n' +
289+
' at foo (/path/to/file.js:10:5)\n' +
290+
' at bar (/path/to/file.js:20:3)';
291+
292+
const trace = parse(err);
293+
294+
// Should NOT have a source location frame prepended
295+
assert.strictEqual(trace.length, 2);
296+
assert.strictEqual(trace[0].getFunctionName(), 'foo');
297+
assert.strictEqual(trace[0].getFileName(), '/path/to/file.js');
298+
assert.strictEqual(trace[1].getFunctionName(), 'bar');
299+
});
300+
301+
it("TypeError stack is not affected by source location detection", () => {
302+
const err = {};
303+
err.stack =
304+
'TypeError: Cannot read property x of undefined\n' +
305+
' at Object.method (/app/index.js:5:10)';
306+
307+
const trace = parse(err);
308+
309+
assert.strictEqual(trace.length, 1);
310+
assert.strictEqual(trace[0].getFunctionName(), 'Object.method');
311+
});
312+
313+
it("RangeError stack is not affected by source location detection", () => {
314+
const err = {};
315+
err.stack =
316+
'RangeError: Maximum call stack size exceeded\n' +
317+
' at recursive (/app/index.js:3:5)';
318+
319+
const trace = parse(err);
320+
321+
assert.strictEqual(trace.length, 1);
322+
assert.strictEqual(trace[0].getFunctionName(), 'recursive');
323+
});
324+
325+
it("SyntaxError from require with Windows paths", () => {
326+
const err = {};
327+
err.stack =
328+
'C:\\Users\\dev\\project\\index.js:15\n' +
329+
'const x = @invalid;\n' +
330+
' ^\n' +
331+
'\n' +
332+
'SyntaxError: Invalid or unexpected token\n' +
333+
' at Module._compile (internal/modules/cjs/loader.js:723:23)';
334+
335+
const trace = parse(err);
336+
337+
assert.strictEqual(trace[0].getFileName(), 'C:\\Users\\dev\\project\\index.js');
338+
assert.strictEqual(trace[0].getLineNumber(), 15);
339+
assert.strictEqual(trace[0].getColumnNumber(), null);
340+
assert.strictEqual(trace[0].getFunctionName(), null);
341+
342+
assert.strictEqual(trace[1].getFunctionName(), 'Module._compile');
343+
});
344+
345+
// Edge cases: custom error types that should NOT trigger source location detection
346+
it("custom error type (not ending in Error) is not treated as source loc", () => {
347+
const err = {};
348+
err.stack =
349+
'MyException: /path/to/file.js:10\n' +
350+
' at fn (file.js:1:2)';
351+
352+
const trace = parse(err);
353+
// The ": " in "MyException: ..." marks this as an error message line
354+
assert.strictEqual(trace.length, 1);
355+
assert.strictEqual(trace[0].getFunctionName(), 'fn');
356+
});
357+
358+
it("URL with port number is not treated as source loc", () => {
359+
const err = {};
360+
err.stack =
361+
'http://localhost:3000\n' +
362+
' at fn (file.js:1:2)';
363+
364+
const trace = parse(err);
365+
assert.strictEqual(trace.length, 1);
366+
assert.strictEqual(trace[0].getFunctionName(), 'fn');
367+
});
368+
369+
it("https URL with port number is not treated as source loc", () => {
370+
const err = {};
371+
err.stack =
372+
'https://example.com:443\n' +
373+
' at fn (file.js:1:2)';
374+
375+
const trace = parse(err);
376+
assert.strictEqual(trace.length, 1);
377+
assert.strictEqual(trace[0].getFunctionName(), 'fn');
378+
});
379+
380+
it("error message containing colon and digits is not treated as source loc", () => {
381+
const err = {};
382+
err.stack =
383+
'MyFault: at line:5\n' +
384+
' at fn (file.js:1:2)';
385+
386+
const trace = parse(err);
387+
assert.strictEqual(trace.length, 1);
388+
assert.strictEqual(trace[0].getFunctionName(), 'fn');
389+
});
390+
391+
it("empty first line does not produce source loc frame", () => {
392+
const err = {};
393+
err.stack =
394+
'\n' +
395+
' at fn (file.js:1:2)';
396+
397+
const trace = parse(err);
398+
assert.strictEqual(trace.length, 1);
399+
assert.strictEqual(trace[0].getFunctionName(), 'fn');
400+
});
401+
402+
// Behavioral equivalence: forEach produces same results as map+filter would
403+
it("non-parseable lines are filtered (no undefined/null in output)", () => {
404+
const err = {};
405+
err.stack =
406+
'Error: test\n' +
407+
' some junk line\n' +
408+
' more junk\n' +
409+
' at fn (file.js:1:2)\n' +
410+
' ~~~not valid~~~\n' +
411+
' at bar (file.js:5:3)';
412+
413+
const trace = parse(err);
414+
// Only valid frames should appear, no undefined entries
415+
assert.strictEqual(trace.length, 2);
416+
assert.strictEqual(trace[0].getFunctionName(), 'fn');
417+
assert.strictEqual(trace[1].getFunctionName(), 'bar');
418+
// Verify no entry is undefined or null
419+
trace.forEach(site => {
420+
assert.notStrictEqual(site, undefined);
421+
assert.notStrictEqual(site, null);
422+
});
423+
});
424+
425+
it("returns a new array each call (no shared state)", () => {
426+
const err = { stack: 'Error: x\n at fn (file.js:1:2)' };
427+
const trace1 = parse(err);
428+
const trace2 = parse(err);
429+
assert.notStrictEqual(trace1, trace2);
430+
assert.deepStrictEqual(trace1.length, trace2.length);
431+
});
432+
433+
// Regex performance: ensure no catastrophic backtracking
434+
it("parse regex does not hang on adversarial input", () => {
435+
// Construct a line designed to stress the regex with repeated patterns
436+
const adversarial = ' at ' + 'a'.repeat(10000) + '(' + 'b'.repeat(10000) + ')';
437+
const err = { stack: 'Error: test\n' + adversarial };
438+
const start = performance.now();
439+
const trace = parse(err);
440+
const elapsed = performance.now() - start;
441+
// Should complete in well under 100ms even for 20KB lines
442+
assert(elapsed < 100, `parse took ${elapsed}ms on adversarial input`);
443+
assert.strictEqual(trace.length, 1);
444+
});
445+
446+
it("source loc regex does not hang on adversarial first line", () => {
447+
// Construct a first line designed to stress /^(.+?):(\d+)(?::(\d+))?$/
448+
const adversarial = 'a'.repeat(50000) + ':1';
449+
const err = { stack: adversarial + '\n at fn (file.js:1:2)' };
450+
const start = performance.now();
451+
const trace = parse(err);
452+
const elapsed = performance.now() - start;
453+
assert(elapsed < 100, `parse took ${elapsed}ms on adversarial source loc`);
454+
// The long path should be captured as source loc (no ": " or "://")
455+
assert.strictEqual(trace[0].getFileName(), 'a'.repeat(50000));
456+
assert.strictEqual(trace[0].getLineNumber(), 1);
457+
});
215458
});

0 commit comments

Comments
 (0)