Skip to content
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

toThrowError() doesn’t catch async throws #2262

Closed
6 tasks done
drwpow opened this issue Nov 3, 2022 · 7 comments
Closed
6 tasks done

toThrowError() doesn’t catch async throws #2262

drwpow opened this issue Nov 3, 2022 · 7 comments
Labels
documentation Improvements or additions to documentation pr welcome

Comments

@drwpow
Copy link
Contributor

drwpow commented Nov 3, 2022

Describe the bug

When expect(…).toThrowError() is used with an async function, Vitest doesn’t catch it.

test('async throws', async () => {
  await expect(async () => {
    await asyncFunctionThatThrows();
  }).toThrowError();
});

Vitest reports that the function didn’t throw, and there was an uncaught exception that happened elsewhere.

AssertionError: expected [Function] to throw an error
 ❯ test.test.ts:20:7
     18|     await expect(async () => {
     19|       await asyncFunctionThatThrows();
     20|     }).toThrowError();
       |       ^
     21|   });
     22| });

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

Test Files  1 failed (1)
     Tests  1 failed | 1 passed (2)
  Start at  17:59:00
  Duration  1.54s (transform 522ms, setup 0ms, collect 104ms, tests 11ms)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: Aaah!

Note that expect.toThrowError() works just fine with synchronous throws.

I’d love to try submitting a PR to fix this if desired 🙏

Reproduction

https://github.com/drwpow/vitest-throw-repro

System Info

System:
    OS: macOS 13.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 90.48 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.12.0 - ~/Library/Caches/fnm_multishells/56571_1667433083650/bin/node
    npm: 8.19.2 - ~/Library/Caches/fnm_multishells/56571_1667433083650/bin/npm
  Browsers:
    Chrome: 107.0.5304.87
    Firefox: 105.0.2
    Firefox Developer Edition: 107.0
    Safari: 16.1
  npmPackages:
    vitest: ^0.24.5 => 0.24.5

Used Package Manager

npm

Validations

@drwpow
Copy link
Contributor Author

drwpow commented Nov 3, 2022

Actually, I have found that this works, and there are existing tests for this:

  await expect((async () => {
    await asyncFunctionThatThrows();
  })()).rejects.toThrowError();

But this seems to be a deviation from the Jest API. If this is the intended way to run async expectations, could this be added to the docs?

Or is there something here about allowing the simpler syntax for parity with Jest?

I do think there is some bug here, because expect(async () => {…}) actually does invoke the function, which then throws an error. The function wasn’t called on its own. So there might be some fix to allow expect to handle errors from functions it calls 🤔

@sheremet-va
Copy link
Member

sheremet-va commented Nov 3, 2022

This is expected behaviour. If you look at type definition, .toThrowError doesn't return a Promise, so await on him yields nothing.

But this seems to be a deviation from the Jest API. If this is the intended way to run async expectations, could this be added to the docs?

It also doesn't work in Jest, and not a deviation from Jest API. You can rewrite your example as:

await expect(asyncFunctionThatThrows).rejects.toThrowError();

We can add a note in docs on how to test it asynchronously, and redirect to .rejects helper section. And we can also add a helper message in expected [Function] to throw an error to use .rejects.

@sheremet-va sheremet-va added wontfix documentation Improvements or additions to documentation and removed wontfix labels Nov 3, 2022
@drwpow
Copy link
Contributor Author

drwpow commented Nov 3, 2022

Sounds good to me. And thanks for double-checking.

Feel free to close this issue as it seems to be documentation then (unless you want to track that via this issue).

Also I am glad to open up a PR for that documentation if desired!

@sheremet-va
Copy link
Member

Sounds good to me. And thanks for double-checking.

Feel free to close this issue as it seems to be documentation then (unless you want to track that via this issue).

Also I am glad to open up a PR for that documentation if desired!

PR is always welcome 👍

@drwpow
Copy link
Contributor Author

drwpow commented Nov 16, 2022

Sorry for the delay on this. I’m still planning on opening a PR to update the docs. Before I do, I wanted to double-check the current behavior of Jest regarding async expect behavior in a few scenarios and make sure Vitest matched. Semi-related to this PR, I was actually calling process.exit(1) in some codepaths in my async function, and that was causing weirdness in Vitest (understandably so, but I wanted to try some uncommon scenarios like that in Jest just to see how it handled it).

Anyway, I just haven’t set time aside for that but will soon. I want to make sure my knowledge (and the docs) are accurate

@MrShnaider
Copy link

Hello. Faced the same problem and found a solution with rejects only on some random site. It would be great if the documentation specifically mentioned rejects as a way of working with asynchronous functions and in the absence of exceptions, the user was reminded that he might have forgotten rejects

@drwpow
Copy link
Contributor Author

drwpow commented Dec 6, 2022

Spent a little time comparing Vitest to Jest, and at least in simple examples I couldn’t find any discrepancy between how both handle .toThrowError() in both sync and async functions. I probably just misremembered how Jest works, but in the off-chance it’s a more edge-case bug that would be a separate issue entirely. Added a PR with some minor additions to the docs. Will close this issue whether that merges or not

@drwpow drwpow closed this as completed Dec 6, 2022
sheremet-va pushed a commit that referenced this issue Jan 9, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation pr welcome
Projects
None yet
Development

No branches or pull requests

3 participants