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

Detect hanging async operations #3119

Open
4 tasks done
cyco130 opened this issue Apr 2, 2023 · 4 comments · May be fixed by #5395
Open
4 tasks done

Detect hanging async operations #3119

cyco130 opened this issue Apr 2, 2023 · 4 comments · May be fixed by #5395
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority) pr welcome

Comments

@cyco130
Copy link
Contributor

cyco130 commented Apr 2, 2023

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

@dsumac
Copy link

dsumac commented Dec 7, 2023

Do you have some feedbacks on this issue ?

@sheremet-va
Copy link
Member

sheremet-va commented Dec 7, 2023

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

async onBeforeRunTask(test: Test) {

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:

const { VitestTestRunner, NodeBenchmarkRunner } = await executor.executeFile(runnersFile)

@sheremet-va sheremet-va added enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority) pr welcome labels Dec 7, 2023
@tkrotoff
Copy link

tkrotoff commented Dec 7, 2023

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)

@sheremet-va
Copy link
Member

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

@fenghan34 fenghan34 linked a pull request Mar 17, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority) pr welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants