Skip to content

Always remove node internals from stacktraces #4695

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.

Sign up for GitHub

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

Merged
cpojer merged 7 commits into from
Oct 15, 2017
Merged

Always remove node internals from stacktraces #4695

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
81d3533
Always remove node internals from stacktraces
SimenB Oct 14, 2017
1596e89
Update link to PR in changelog
SimenB Oct 14, 2017
4472f9b
Fix browser test
SimenB Oct 14, 2017
f097ece
Remove weird next_tick trace just in integration tests
SimenB Oct 14, 2017
3c294ee
Rename some regexpes
SimenB Oct 14, 2017
b4bfc53
Revert "Remove weird next_tick trace just in integration tests"
SimenB Oct 14, 2017
c68e670
Note code that can be delete when we drop support fo node 4
SimenB Oct 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Revert "Remove weird next_tick trace just in integration tests" 
This reverts commit f097ece.
  • Loading branch information
@SimenB
SimenB committed Oct 14, 2017
commit b4bfc534bac5674fe1e46b2828e7c7c0014672dd
5 changes: 2 additions & 3 deletions integration_tests/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,12 @@ const extractSummary = (stdout: string) => {
// different versions of Node print different stack traces. This function
// unifies their output to make it possible to snapshot them.
const cleanupStackTrace = (output: string) => {
return output
.replace(/\n.*at.*next_tick\.js.*$/gm, '')
.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1');
return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1');
};

module.exports = {
cleanup,
cleanupStackTrace,
copyDir,
createEmptyPackage,
extractSummary,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-message-util/src/__tests__/messages.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const assertionStack =
at promise.then (node_modules/jest-jasmine2/build/queue_runner.js:74:39)
at <anonymous>
at process._tickCallback (internal/process/next_tick.js:188:7)
at internal/process/next_tick.js:188:7
Copy link
Member Author

@SimenB SimenB Oct 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this last one I just saw in the integration tests. It should be the same as the one above it. Ideas?

Copy link
Member Author

@SimenB SimenB Oct 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to come from the remaining replace in cleanupStackTrace in integration_tests/utils.js. Not sure how to handle it

Copy link
Member Author

@SimenB SimenB Oct 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we really want to do it, I can move this next_tick part down there, no reason to have it in message-util

Copy link
Member Author

@SimenB SimenB Oct 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figured it out, this is node 4 missing the calling function's name.

`;

it('should exclude jasmine from stack trace for Unix paths.', () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/jest-message-util/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import StackUtils from 'stack-utils';
let nodeInternals = [];

try {
nodeInternals = StackUtils.nodeInternals();
nodeInternals = StackUtils.nodeInternals()
// Somehow we get a trace without the `process._tickCallback` part
.concat(new RegExp('internal/process/next_tick.js'));
} catch (e) {
// `StackUtils.nodeInternals()` fails in browsers. We don't need to remove
// node internals in the browser though, so no issue.
Expand Down