-
-
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
Jest does not allow asynchronous catching of rejected promises #6028
Comments
Duplicate of #5311 (although this has a better description 🙂) |
@SimenB this is not a duplicate. #5311 is about a warning and what looks to me like a misunderstanding. In @gaearon's code, This issue is a serious bug, it means one can't write code which asynchronously catches promise rejections. This is entirely unexpected. |
Going back to jest 20 from 24 solves this, so it's obviously a regression. |
It has the same underlying cause - we added our own 'unhandled rejection' handler. The change is a good one, as a random unhandled promise will fail your test (like a sync error) instead of either being swallowed or just printed as a warning. I think it's way more more often the case you want to fail tests on unhandled promise rejections than testing handlers. However, I think we should allow disabling it on a test by test basis if you explicitly want to test rejection handlers. I think that can be tracked in #5311, or do you think it's orthogonal? |
I'm having the same problem. Consider the following test: it('terminates the test prematurely', async () => {
const promise = new Promise((resolve, reject) => setTimeout(() => reject(new Error('Oops')), 0));
await new Promise(resolve => setTimeout(resolve, 10));
await expect(promise).rejects.toThrow('Oops');
}); This creates a promise, does some asynchronous stuff, then expects that the promise fails. But execution never reaches the assertion. As soon as the promise rejects (while waiting for the timeout), the unit test is terminated and marked as failing. I understand that #5311 has a similar cause, but I believe these are two different issues. My understanding is that #5311 describes the case that at the end of the test, promises are still dangling. This should be avoided. This issue, in contrast, describes the case that during test execution, a promise is not immediately subscribed to. This, in my opinion, is a perfectly valid scenario that should work. |
What's more, the code may not even be part of the test suite, but of the system under test. Consider: code-under-test.js export async function codeUnderTest() {
const promise = new Promise((resolve, reject) => setTimeout(() => reject(new Error('Oops')), 0));
await new Promise(resolve => setTimeout(resolve, 10));
return promise;
} test.js import { codeUnderTest } from '../src/code-under-test.js';
describe('codeUnderTest', () => {
it('terminates the test prematurely', async () => {
await expect(codeUnderTest()).rejects.toThrow('Oops');
});
}); The function |
After much frustration, we were able to work around this issue by adding a dummy catch to the promise. it('terminates the test prematurely', async () => {
const promise = new Promise((resolve, reject) => setTimeout(() => reject(new Error('Oops')), 0));
promise.catch(err => { return; });
await new Promise(resolve => setTimeout(resolve, 10));
await expect(promise).rejects.toThrow('Oops');
}); This will not solve the issue in an imported function, but should at least band-aid the problem in tests. |
Nice idea! I reworked it into a function defuse(promise) {
promise.catch(() => {});
return promise;
}
it('terminates the test prematurely', async () => {
const promise = defuse(new Promise((resolve, reject) => setTimeout(() => reject(new Error('Oops')), 0)));
await new Promise(resolve => setTimeout(resolve, 10));
await expect(promise).rejects.toThrow('Oops');
}); Still, this is a workaround for a problem that I believe should be fixed. |
Hi @DanielSWolf would you mind explaining how/why this fixes the issue? I just wrote a |
@Rexman17 Let's walk through the function promise.catch(() => {}); This line creates a new promise that catches and swallows any rejection (error) by the original promise. If the original promise succeeds with a value, the new promise succeeds with the same value. If the original promise fails, the new promise still succeeds, but with But notice that this isn't what we return: return promise; What we return, instead, is the original promise. This means that any code using the "defused" Promise will behave exactly as if it were using the original promise. After all, it is the same promise. The only difference is that after the call to Only that, apparently, Jest does some cheating and checks whether every promise has a |
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
Jest version:
22.4.3
(But was introduced in21.x
)Config: None (all defaults)
Node version:
6.11.5
,8.9.4
Jest does not allow you to asynchronously attach a
.catch()
to a promise, even if it's added before the test itself is finished. The only difference between these two tests is that the catch is added within a setTimeout() in the second one.In Jest
20.x
, only warnings are printed, and the test does not fail(Was originally mentioned in #3251)
The text was updated successfully, but these errors were encountered: