Repository navigation
fix: skip file-less stack frames in extractSource #3486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
034413b
3cf261f
314370a
7216266
a38d243
17a5422
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import path from 'node:path'; | ||
| import {pathToFileURL} from 'node:url'; | ||
|
|
||
| import {test} from 'tap'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New tests should be under
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved in the earlier follow-up — coverage now lives in |
||
|
|
||
| import serializeError from '../lib/serialize-error.js'; | ||
|
|
||
| test('extractSource skips stack frames without a file path', t => { | ||
| const testFilePath = path.resolve('test-fixture.js'); | ||
| const testFile = pathToFileURL(testFilePath).toString(); | ||
| const error = new Error('boom'); | ||
| // Stack-utils leaves `file` undefined for native frames; previously | ||
| // normalizeFile(undefined) threw and hid the original test failure. | ||
| error.stack = `Error: boom | ||
| at Function.foo (native) | ||
| at Object.<anonymous> (${testFilePath}:10:5)`; | ||
|
|
||
| const result = serializeError(error, {testFile}); | ||
| t.equal(result.type, 'native'); | ||
| t.same(result.source, { | ||
| isDependency: false, | ||
| isWithinProject: true, | ||
| file: testFile, | ||
| line: 10, | ||
| }); | ||
| t.end(); | ||
| }); | ||
|
|
||
| test('extractSource skips anonymous file paths that are not the test file', t => { | ||
| const testFilePath = path.resolve('test-fixture.js'); | ||
| const testFile = pathToFileURL(testFilePath).toString(); | ||
| const error = new Error('boom'); | ||
| error.stack = `Error: boom | ||
| at Object.<anonymous> (<anonymous>:1:2) | ||
| at Object.<anonymous> (${testFilePath}:4:1)`; | ||
|
|
||
| const result = serializeError(error, {testFile}); | ||
| t.equal(result.type, 'native'); | ||
| t.same(result.source, { | ||
| isDependency: false, | ||
| isWithinProject: true, | ||
| file: testFile, | ||
| line: 4, | ||
| }); | ||
| t.end(); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm cool with not having the line numbers in the logs (unrelated though it is to what this PR fixes), but this test adds no value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped the reporter sanitizer and the edge-case log refreshes in 7216266 — they were unrelated to the serialize-error fix. The PR is now only
lib/serialize-error.js+test/serialize-error.js.