-
-
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
Unhandled async errors crash Jest. #2059
Comments
Oh, it might have been because I didn't return the promise but instead am using Jasmine's async work. Nevertheless, we should hook up an unhandled promise handler in Jest. |
Currently we use in our tests a utility to work around this in a quite cumbersome way: /**
* In async tests, JEST will die (in watch mode) if an exception is thrown from a callback. This utility will catch
* the errors instead and report the test as failed in these case *
*
* @param {jest.DoneCallback} done
* @param {T} callback
* @returns {T}
*/
export function catchErrors<T extends Function>(done: jest.DoneCallback, callback: T): T {
return function() {
try {
callback.apply(this, arguments)
} catch (e) {
done.fail(e)
}
} as any as T
} which then can be used as follows: describe("suite", () => {
it("should handle async exceptions", (done) => {
setTimeout(catchErrors(done, (err, res) => {
throw "Catched by catchErrors so that our tests properly fail!"
done()
}), 100)
})
}) Any cleaner approach to this would be appreciated! |
this sounds somewhat similar to #1873, where |
Any news on this? I also have a repro repo here if that can help. |
This is some huge drawback for me :( Trying to find a way to circumvent this properly... |
this is quite frustrating as I have no idea where it's dying |
@mweststrate this should be in the jest core. Seriously, this has been lying here for two months and no one opened a PR yet? 😖 |
@capaj seems like you are on the hook to make that PR. Jest is an open source project, if you'd like something to be fixed, please start contributing. The attitude you are showing on this issue tracker is not appropriate. |
@cpojer sorry I am just very enthusiastic about my favorite OSS tools. I wasn't aware that displaying negative emotions about negative things is prohibited in jest repo. Next time, I will try to write my comments like a robot, deprived of any emotions. |
I'd prefer if you channeled your enthusiasm into code which will help make this project better for everyone or channel it into productive conversation. Negative comments don't help anybody; if that's the only thing you are contributing to this project I'd prefer to see you using a different testing platform. |
@cpojer I beg to disagree, but that's a discussion better suited for some other place/platform |
That's ok, we can disagree on this but the Jest project doesn't need your negativity and we won't accept this behavior here. |
In mocha, the done callback takes an optional error (standard Node.js cb style). If you call done with a truthy value then it will be used as an error. promise.then(x => {
// do something
done(); // successful
}).catch(err => {
// do something with error
done(err); // hand error to test runner
}); |
Having a look through this there isn't a reproduction example, so I thought I'd include one. In here the first test will fail reporting
while the other two tests correctly reporting back the failed expectation. Correctly works in Jasmine. describe("Error Reporting", function() {
it("Async", function(done) {
setTimeout(function() {
expect(true).toBe(false);
done();
}, 100);
});
it("Sync", function() {
expect(true).toBe(false);
});
it("Fake Async", function(done) {
expect(true).toBe(false);
done();
});
}); |
I submitted #1897 that aslo included a reproducible example: https://github.com/evansiroky/jest-exec-bug |
Due to jestjs/jest#2059, turns out we need those try/catch blocks I tried to remove in the previous PR. Partially reverts lerna#682
Due to jestjs/jest#2059, turns out we need those try/catch blocks I tried to remove in the previous PR. Partially reverts #682
@jeffbski indeed thats the most convenient way of doing this! I thought we would be able |
@aaronabramov the PR should be ready now, it would be great to get some review on it so we can get this shipped :) |
* wip: start handling async errors Ref #2059 * Implement basic cancelation * Improve cancelation * lint and flow happy, more tests * snapshot updates * happy tests * add tests for before hooks * log errors if they escape us * use p-cancelable * improve based on CR
This is part of Jest 21 thanks to @dignifiedquire. Published |
@dignifiedquire @cpojer I still got timeout for the following test. I am sure I installed version 21.1.0 and I also tried version 21.0.2. test.spec.js it('exception promise', (done) => {
setTimeout(()=>{
expect(true).toEqual(false)
done()
}, 10)
}) package.json {
"name": "testJest",
"version": "1.0.0",
"main": "index.js",
"license": "MIT",
"dependencies": {
"jest": "21.1.0"
},
"scripts": {
"test": "jest --debug"
}
} |
Looks like ^ fails when using Using
Using
Minimal repro: https://github.com/kjbekkelund/jest-timeout-issue-jsdom |
@kjbekkelund yeah, thank you. |
I think that last case might be fixed in #4669 |
@dignifiedquire hey there. In which Jest version that was fixed? Using Jest 21.1.6 and it's still there. My particular case is there https://stackoverflow.com/a/47760009/5086732, crashing if remove |
I'm still seeing this with v20.0.4. In my case with a check inside a redux store reducer. |
Seems to be a gift that gives on giving... |
Still happening in 22? |
Ah, is working in 22.0.5. Great, thanks! |
I am still experiencing this in 22.1.1 in the following test: it('replies with the correct message': done => {
// Initialise a bot with the MS botbuilder framework
bot.on('send', message => {
expect(message.text).toEqual('Is everything ok?');
done();
});
}); |
Can you provide a repro I can pull down to test? |
Thanks for the quick reply. Yes, will do so within the next 30 mins. |
Here is a repo to reproduce the issue: https://github.com/rawroland/async-jest-test. |
@rawroland that's expected, you're basically seeing #3917. I'd recommend using promise instead of callback based code, then you don't have to worry about that sort of code flow. E.g. it('replies with the correct message', async () => {
// Initialise a bot with the MS botbuilder framework
const result = await new Promise(resolve => bot.on('send', resolve));
expect(result.text).toEqual('Is everything ok?');
}); |
Thank you for the information. |
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. |
On master, I changed one of the tests in jest-haste-map from
toBeDefined()
or something tonot.toBeDefined()
. It would then go on to crash Jest:@DmitriiAbramov what have we done?
The text was updated successfully, but these errors were encountered: