-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Do not filter out stack trace for unhandled rejections #10784
Comments
+1 |
Also experienced this in 15 and the downgrade fixed it. |
This is such a frustrating default behavior. Turns out the bug wasn't caused by my code changes, but an upgraded dependency in unrelated code... I had to use VS Code debugger to trap the error. It was pretty frustrating to find all the line number details I'd expect were there. They were simply dropped by node+jest's output. Is there a way to let users know the risk of using Or is there a node.js option to make the crash behavior less user-hostile? |
+1 |
1 similar comment
+1 |
Does |
Thanks your hint + (https://jestjs.io/docs/troubleshooting#debugging-in-vs-code) helped me solve my issue. I was using |
No, this flag doesn't help. What kind of flag is that? I can't find any mention of it anywhere? |
I'm using handling it with:
in jest.setup.js |
Currently, you can just use |
@LvChengbin |
|
@vicary it should warn about them, but still keep running. If you want it to warn, and then fail your test suite with a clear error that you can more easily track down, use the For those using jest through npm scripts, that means using something like this:
Without the unhandled rejection flag, you'd see this:
But using
|
@Pomax Thanks for the advise. Give that I want the test to fail AND have a precise stack trace so I can fix it, instead of letting the it slide and call it a day. According to the docs, the description of Does it makes sense? |
if |
This behavior looks consistent with the default value for Node's
This makes it seem like Jest has not attached an |
We found a solution, although I am sure it has a performance impact.
They do attach it. But if you look carefully they restore it after the tests are complete. So what is happening is the tests are completing, they are restoring the (non-existant) event handlers, and then node is blowing up. Since the promise is rejecting after the test suite finishes. We found a workaround, by flushing the promises after each test, the error is forced to happen exactly where the test fails, and it even marks the test as failed. I am sure it will have a performance impact, so I am testing that a bit. https://www.npmjs.com/package/flush-promises
|
@timritzer ah, nice find! That matches up with the fact that the following code will produce an error handled by Jest when it('example test', async () => {
Promise.reject(new Error('error'));
// Comment out the following line to see an unhandled rejection error
await new Promise(resolve => setTimeout(resolve, 0));
}); |
Yep! Also just did some before/after testing on our admittedly small test base of ~600 tests and 5 snapshot tests. Performance difference was pretty minimal. Ultimately it's just an empty promise that resolves so I can't imagine it will have too much overhead. |
I did a little more digging - per the Node source code, it looks like Node should be printing a proper stack trace for uncaught exceptions, as long as the rejection reason is an instance of However, for some reason, it seems like errors that are thrown in Jest code aren't actually errors. You can reproduce this easily with the node interpreter. Compare the output of the following scripts:
The former (which is an actual error) includes a very nice and easy to understand stack trace; the latter (which is not an error) shows the reference to Node internals that the original report in this thread shows. I'm not sure why an error isn't seen as Interestingly, if you add the following to a script referenced from process.on('unhandledRejection', (reason) => {
throw reason;
}); |
To explain the "why" at least: Promises do not throw errors on rejection (and never have). Instead, all they do is generate a new, completed promise with the rejected flag set and the rejection data bound so you can get that data out. Rejections themselves are in fact the opposite of an error: they're one of the two well-defined ways in which a promise is expected to complete. However, if something doesn't handle a promise rejection, that's an error and you get an |
I think I've narrowed this down to an issue with Jest's const require('console');
console.log('Error prototypes equal? ' + String(Error === global.Error)); When I run Jest, I see the output As for a fix... let's consider the following simple test case: describe('index', () => {
it('has unhandled rejection', () => {
Promise.reject(new Error('error in test'));
});
}); If we run that with the latest published version of Jest, we'll see the Now, let's add the following line below this line in the Node environment implementation: global.Error = Error Now when we run the above test, we see the actual stack trace:
This isn't perfect - for instance, it doesn't handle someone rejecting a promise with a
Copying over the FWIW, it looks like there's precedence for this kind of change. For instance, #7626 copied over the @SimenB does this seem like the right fix? I'd be happy to make a PR if so. |
#5066) #### Details This PR adds a new setup step to each of our test suites which forces Jest to flush all `Promise`s in a globally applied `afterEach` fixture. This forces Jest to flush any pending resolved Promises that a test might have forgotten to handle *before* Jest considers that test to be completed. That way, if a test leaks a Promise, the resulting error message gets logged as a failure in the responsible test and includes a nice, Jest-formatted stack, instead of test "passing" and the error as a Node UnhandledProjmiseRejectionWarning with no stack or test context. This is intended to work in tandem with #5061, allowing it to go in as a last-resort while mitigating the poor logging that we get if it's actually triggered (by avoiding having it need to trigger in the common case) ##### Motivation Avoids silent test failures (eg, the one that currently exists in `injector-controller.test.ts`) ##### Context jestjs/jest#10784 (comment) #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [n/a] Addresses an existing issue: #0000 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
I've opened a bug for nodejs: nodejs/node#41676 |
Hey, saw this in the Node repo Pull request based on @ItamarGronich 's issue in Node: nodejs/node#41682 We likely won't fix the issue of primitives throwing (also raised there, at least I'm personally -1 but other members may feel differently) but I've altered the check for unhandled rejection error logging to check if the error has a As mentioned previously you can use the process.on('unhandledRejection', (reason) => {
console.log(reason); // log the reason including the stack trace
throw e;
}); |
Even if that PR might land soon, it might be worth updating the docs for now to explain this use, in an easily discoverable location with links to it from other obvious places people will be looking. Worst case, the docs can be reverted a few days from now, but some Node PRs end up taking months of back and forth, and then having those docs updated will make a huge difference to a lot of people =) |
The PR landed yesterday and will go out with the next v12/v14/v16/v17 versions. Probably a good idea to update the docs regardless. |
Happy to take a docs PR 🙂 |
This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
Not stale |
Stale, stale, this was fixed for every non end-of-life version of Node.js :) @cameron-martin |
Oops, somebody close it then |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
🐛 Bug Report
In strict unhandled rejections mode (https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode), the stack trace of unhandled rejection is not displayed in the console. This mode is default in Node.js 15.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The stack trace is displayed in the console.
Link to repl or repo (highly encouraged)
https://github.com/vkrol/jest-unhandled-rejection-stack-trace
envinfo
The text was updated successfully, but these errors were encountered: