vitest icon indicating copy to clipboard operation
vitest copied to clipboard

Detect hanging async operations

Open cyco130 opened this issue 2 years ago • 8 comments

Clear and concise description of the problem

As a developer using Vitest I want to be able to detect if an async operation that was started during a test case hasn't completed at the end of that test so that I can catch hanging promise bugs. I am willing to contribute a PR if there is interest.

Suggested solution

We could provide a --detect-async-leaks flag to turn on this type of detection. Node.js's builtin async_hooks module provides the necessary functionality

Alternative

Alternatively, we could make this the default and provide an option to turn it off.

Additional context

For example in Deno the following test fails with the message "Test case is leaking async ops":

Deno.test("hanging interval", () => {
  setInterval(() => {}, 1000);
});

This helped me catch a few particularly hard to find bugs in a streaming form data parser I was working on. Web streams are particularly prone to this type of error but there are many other cases where such detection would be useful.

Validations

cyco130 avatar Apr 02 '23 08:04 cyco130

Do you have some feedbacks on this issue ?

dsumac avatar Dec 07 '23 10:12 dsumac

Sounds like a nice feature. We can probably extend runner somewhere here:

https://github.com/vitest-dev/vitest/blob/7006bb367494536e2ecf762a5636e509734e43e5/packages/vitest/src/runtime/runners/test.ts#L54

Since this is Node.js-only feature, we would need to extend this class in a separate file so as not to break browser runner which also relies on this class.

In Node.js it is imported here:

https://github.com/vitest-dev/vitest/blob/7006bb367494536e2ecf762a5636e509734e43e5/packages/vitest/src/runtime/runners/index.ts#L15

sheremet-va avatar Dec 07 '23 10:12 sheremet-va

Here a comparison with Jest (no CLI option or special config required):

import { test } from "vitest";

// Jest output: "ELIFECYCLE Test failed. See above for more details."
test("error after tests are done", () => {
  setTimeout(() => {
    throw new Error("after tests are done");
  }, 100);
});

// Jest output:
// "Jest did not exit one second after the test run has completed.
//  This usually means that there are asynchronous operations that weren't stopped in your tests.
//  Consider running Jest with `--detectOpenHandles` to troubleshoot this issue."
// "Jest has detected the following 1 open handle potentially keeping Jest from exiting: [...]"
test("open handles", () => {
  setInterval(() => {}, 100);
});

// Jest output: "Cannot log after tests are done. Did you forget to wait for something async in your test?"
test("console usage after tests are done", () => {
  setTimeout(() => {
    console.info("after tests are done");
  }, 100);
});

Vitest won't report any issue while all these tests fail with Jest (modulo this issue)

node:test & Mocha also detect those cases (without any particular CLI option or config)

tkrotoff avatar Dec 07 '23 11:12 tkrotoff

node:test & Mocha also detect those cases (without any particular CLI option or config)

Cannot confirm that this is true. In mocha I get the error "after tests are done" which does change processExit, but doesn't print any information about leaking. node:test just hangs for me (probably because of setInterval).

mocha

image

node:test (Node 20.11.0)

image

sheremet-va avatar Jan 19 '24 20:01 sheremet-va

Hey, this is currently blocking adoption of Vitest for one of our projects. If not to much to ask, would love to know:

  1. How likely is this issue to being acted on by the core maintainers
  2. If it's not being acted upon because it's not seen as valuable, could you share a few reasons you think why that is. It might help shift our team's thinking to stop relying on that Jest feature.
  3. If it's not being acted upon because of core team time constraints, would be a PR be welcome for this?

Thanks!

jasonkuhrt avatar Dec 11 '24 17:12 jasonkuhrt

Hey, this is currently blocking adoption of Vitest for one of our projects. If not to much to ask, would love to know:

  1. How likely is this issue to being acted on by the core maintainers
  2. If it's not being acted upon because it's not seen as valuable, could you share a few reasons you think why that is. It might help shift our team's thinking to stop relying on that Jest feature.
  3. If it's not being acted upon because of core team time constraints, would be a PR be welcome for this?

Thanks!

I'm not a maintainer, but:

  1. Unlikely, there's an existing PR and this is only p2.
  2. I can't speak for the Vitest team, but I'd guess that there's just more important stuff for the them to focus on, like stabilizing browser mode and the Node API.
  3. It has the PR accepted label, so yes.

lishaduck avatar Dec 11 '24 17:12 lishaduck

This will be possible after https://github.com/vitest-dev/vitest/pull/8705

AriPerkkio avatar Oct 20 '25 19:10 AriPerkkio