Skip to content

Commit

Permalink
Always remove node internals from stacktraces (#4695)
Browse files Browse the repository at this point in the history
* Always remove node internals from stacktraces

* Update link to PR in changelog

* Fix browser test

* Remove weird next_tick trace just in integration tests

* Rename some regexpes

* Revert "Remove weird next_tick trace just in integration tests"

This reverts commit f097ece.

* Note code that can be delete when we drop support fo node 4
  • Loading branch information
SimenB authored and cpojer committed Oct 15, 2017
1 parent 291e836 commit 7793554
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 16 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
* `[jest-editor-support]` Fix editor support test for node 4 ([#4640](https://github.com/facebook/jest/pull/4640))
* `[jest-mock]` Support mocking constructor in `mockImplementationOnce` ([#4599](https://github.com/facebook/jest/pull/4599))
* `[jest-runtime]` Fix manual user mocks not working with custom resolver ([#4489](https://github.com/facebook/jest/pull/4489))
* `[jest-runtime]` Move `babel-core` to peer dependenies so it works with Babel 7 ([#4557](https://github.com/facebook/jest/pull/4557))
* `[jest-runtime]` Move `babel-core` to peer dependencies so it works with Babel 7 ([#4557](https://github.com/facebook/jest/pull/4557))
* `[jest-util]` Fix `runOnlyPendingTimers` for `setTimeout` inside `setImmediate` ([#4608](https://github.com/facebook/jest/pull/4608))
* `[jest-message-util]` Always remove node internals from stacktraces ([#4695](https://github.com/facebook/jest/pull/4695))

### Features
* `[jest-environment-*]` [**BREAKING**] Add Async Test Environment APIs, dispose is now teardown ([#4506](https://github.com/facebook/jest/pull/4506))
Expand Down
11 changes: 2 additions & 9 deletions integration_tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,9 @@ const extractSummary = (stdout: string) => {

// different versions of Node print different stack traces. This function
// unifies their output to make it possible to snapshot them.
// TODO: Remove when we drop support for node 4
const cleanupStackTrace = (output: string) => {
return output
.replace(/\n.*at.*timers\.js.*$/gm, '')
.replace(/\n.*at.*assert\.js.*$/gm, '')
.replace(/\n.*at.*node\.js.*$/gm, '')
.replace(/\n.*at.*next_tick\.js.*$/gm, '')
.replace(/\n.*at (new )?Promise \(<anonymous>\).*$/gm, '')
.replace(/\n.*at <anonymous>.*$/gm, '')
.replace(/\n.*at Generator.next \(<anonymous>\).*$/gm, '')
.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1');
return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1');
};

module.exports = {
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-message-util/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"dependencies": {
"chalk": "^2.0.1",
"micromatch": "^2.3.11",
"slash": "^1.0.0"
"slash": "^1.0.0",
"stack-utils": "^1.0.1"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@ exports[`.formatExecError() 1`] = `
"
`;
exports[`formatStackTrace should strip node internals 1`] = `
"<bold><red> <bold>● <bold>Unix test</></>
Expected value to be of type:
\\"number\\"
Received:
\\"\\"
type:
\\"string\\"
<dim> </>
<dim> </>
<dim> <dim>at Object.it (<dim>__tests__/test.js<dim>:8:14)<dim></>
<dim> </>
"
`;
exports[`should exclude jasmine from stack trace for Unix paths. 1`] = `
"<bold><red> <bold>● <bold>Unix test</></>
Expand Down
43 changes: 42 additions & 1 deletion packages/jest-message-util/src/__tests__/messages.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

'use strict';

const {formatResultsErrors, formatExecError} = require('../');
const {formatResultsErrors, formatExecError} = require('..');

const unixStackTrace =
` ` +
Expand All @@ -23,6 +23,27 @@ const unixStackTrace =
at Object.<anonymous> (../jest-jasmine2/build/jasmine-pit.js:35:32)
at attemptAsync (../jest-jasmine2/build/jasmine-2.4.1.js:1919:24)`;

const assertionStack =
' ' +
`
Expected value to be of type:
"number"
Received:
""
type:
"string"
at Object.it (__tests__/test.js:8:14)
at Object.asyncFn (node_modules/jest-jasmine2/build/jasmine_async.js:124:345)
at resolve (node_modules/jest-jasmine2/build/queue_runner.js:46:12)
at Promise (<anonymous>)
at mapper (node_modules/jest-jasmine2/build/queue_runner.js:34:499)
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
`;

it('should exclude jasmine from stack trace for Unix paths.', () => {
const messages = formatResultsErrors(
[
Expand Down Expand Up @@ -62,3 +83,23 @@ it('.formatExecError()', () => {

expect(message).toMatchSnapshot();
});

it('formatStackTrace should strip node internals', () => {
const messages = formatResultsErrors(
[
{
ancestorTitles: [],
failureMessages: [assertionStack],
title: 'Unix test',
},
],
{
rootDir: '',
},
{
noStackTrace: false,
},
);

expect(messages).toMatchSnapshot();
});
48 changes: 44 additions & 4 deletions packages/jest-message-util/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ import path from 'path';
import chalk from 'chalk';
import micromatch from 'micromatch';
import slash from 'slash';
import StackUtils from 'stack-utils';

let nodeInternals = [];

try {
nodeInternals = StackUtils.nodeInternals()
// this is to have the tests be the same in node 4 and node 6.
// TODO: Remove when we drop support for node 4
.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.
}

type StackTraceConfig = {
rootDir: string,
Expand All @@ -26,7 +39,10 @@ type StackTraceOptions = {

// filter for noisy stack trace lines
const JASMINE_IGNORE = /^\s+at(?:(?:.*?vendor\/|jasmine\-)|\s+jasmine\.buildExpectationResult)/;
const STACK_TRACE_IGNORE = /^\s+at.*?jest(-.*?)?(\/|\\)(build|node_modules|packages)(\/|\\)/;
const JEST_INTERNALS_IGNORE = /^\s+at.*?jest(-.*?)?(\/|\\)(build|node_modules|packages)(\/|\\)/;
const ANONYMOUS_FN_IGNORE = /^\s+at <anonymous>.*$/;
const ANONYMOUS_PROMISE_IGNORE = /^\s+at (new )?Promise \(<anonymous>\).*$/;
const ANONYMOUS_GENERATOR_IGNORE = /^\s+at Generator.next \(<anonymous>\).*$/;
const TITLE_INDENT = ' ';
const MESSAGE_INDENT = ' ';
const STACK_INDENT = ' ';
Expand Down Expand Up @@ -106,10 +122,26 @@ const removeInternalStackEntries = (lines, options: StackTraceOptions) => {
let pathCounter = 0;

return lines.filter(line => {
const isPath = STACK_PATH_REGEXP.test(line);
if (!isPath) {
if (ANONYMOUS_FN_IGNORE.test(line)) {
return false;
}

if (ANONYMOUS_PROMISE_IGNORE.test(line)) {
return false;
}

if (ANONYMOUS_GENERATOR_IGNORE.test(line)) {
return false;
}

if (nodeInternals.some(internal => internal.test(line))) {
return false;
}

if (!STACK_PATH_REGEXP.test(line)) {
return true;
}

if (JASMINE_IGNORE.test(line)) {
return false;
}
Expand All @@ -118,7 +150,15 @@ const removeInternalStackEntries = (lines, options: StackTraceOptions) => {
return true; // always keep the first line even if it's from Jest
}

return !(STACK_TRACE_IGNORE.test(line) || options.noStackTrace);
if (options.noStackTrace) {
return false;
}

if (JEST_INTERNALS_IGNORE.test(line)) {
return false;
}

return true;
});
};

Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5444,6 +5444,10 @@ sshpk@^1.7.0:
jsbn "~0.1.0"
tweetnacl "~0.14.0"

stack-utils@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/stack-utils/-/stack-utils-1.0.1.tgz#d4f33ab54e8e38778b0ca5cfd3b3afb12db68620"

state-toggle@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/state-toggle/-/state-toggle-1.0.0.tgz#d20f9a616bb4f0c3b98b91922d25b640aa2bc425"
Expand Down

0 comments on commit 7793554

Please sign in to comment.