fix: add async to onFinish report event

[edimitchel]

Error when using UI with Vitest

(node:50984) UnhandledPromiseRejectionWarning: Error: listen EACCES: permission denied 127.0.0.1:51204
at Server.setupListenHandle [as _listen2] (net.js:1301:21)
at listenInCluster (net.js:1366:12)
at doListen (net.js:1503:7)
at processTicksAndRejections (internal/process/task_queues.js:81:21)
(Use node --trace-warnings ... to show where the warning was created)
(node:50984) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:50984) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future,
promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[netlify]

✔️ Deploy Preview for vitest-dev ready!

🔨 Explore the source changes: 34071b2

🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/61ef39456cebba000791774c

😎 Browse the preview: https://deploy-preview-623--vitest-dev.netlify.app

[Demivan]

Adding async to a function that does not have await should not affect it. What does this do?

[edimitchel]

async behind a function transform the function to a awaitable.

The function report which make sure to send report event needs function which return promise.
See https://github.com/vitest-dev/vitest/blob/main/packages/vitest/src/node/core.ts#L378-L381

[Demivan]

Ok, makes sense that this will return a promise now.
Should this be extended to onCollected and onUserConsoleLog? Those are marked as awaitable in reporter interface too.

[prazdevs]

Hi, tested it on my repo that was crashing on --ui it works perfectly fine now and reruns as expected ! 👍

[mitchelvanbever]

Hi, tested it on my repo that was crashing on --ui it works perfectly fine now and reruns as expected ! 👍

Same here 👌

[edimitchel]

Ok, makes sense that this will return a promise now.
Should this be extended to onCollected and onUserConsoleLog? Those are marked as awaitable in reporter interface too.

Good comment, I think we can remove the Awaitable type.

[sheremet-va]

I think it's better to make them all Awaitable to support async implementation in reporters. Also shouldn't they await Promise.all and not just turn into an async function?

async function onFinished(files?: File[] | undefined) {
  await Promise.all(
    this.clients.map((client) => {
      return client.onFinished?.(files)
    })
  )
}

[antfu]

Actually, events prefixed with on should not expect returns (error on receiver side does not matter to the sender), add it here

vitest/packages/vitest/src/api/setup.ts

Line 104 in c23be10

eventNames: ['onCollected', 'onUserConsoleLog', 'onTaskUpdate'],

Should work

[bjarkihall]

Thanks for the PR!
For those trying to find an issue/PR a text would work better than a screenshot when searching, so I'll just paste it here:

(node:50984) UnhandledPromiseRejectionWarning: Error: listen EACCES: permission denied 127.0.0.1:51204
    at Server.setupListenHandle [as _listen2] (net.js:1301:21)
    at listenInCluster (net.js:1366:12)
    at doListen (net.js:1503:7)
    at processTicksAndRejections (internal/process/task_queues.js:81:21)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:50984) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:50984) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, 
promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[antfu]

I have made the fix in 6b02dea and released it as v0.2.3. Please check if it fixes the error

[bjarkihall]

@antfu I've tried v.0.2.3 on a clean install (using npm) but still get the same error message when running npx vitest --ui
npx vitest itself still works without any problems.
This is likely the same issue as the one reported in #627

The issue provides reproduction steps but it also has a screenshot of where the error is thrown - a Promise.reject is invoked but there's no try/catch for it in when using async/await, could it be that there's no error boundary in the callstack to catch it, so node stops the execution?

[mitchelvanbever]

I have made the fix in 6b02dea and released it as v0.2.3. Please check if it fixes the error

I've now tested two local repo's that had the issue prior to V0.2.3 but after upgrading I'm no longer seeing the messages and/or crashes happening (with and without ui flag).

Stackblitz seems to not complain anymore as well.

[bjarkihall]

@mitchelvanbever on a second look at the original image from @edimitchel, the error messages are indeed not the same, although they both show a similar warning + stacktrace (both caused by unhandled promise rejections).

This one is related to custom reporters.
I'll follow up on this in the issue I linked to --ui (#627).

Any ideas how unhandled promise rejections could be further prevented or at least caught, for the sake of a clearer stacktrace?

[edimitchel]

@antfu what should we do ?

[sheremet-va]

@antfu what should we do ?

I think it should be fixed now. Can you confirm? If so, we may close the PR