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

Flag to allow for graceful exit on hanging process #3909

Closed
4 tasks done
sethreidnz opened this issue Aug 7, 2023 · 7 comments
Closed
4 tasks done

Flag to allow for graceful exit on hanging process #3909

sethreidnz opened this issue Aug 7, 2023 · 7 comments

Comments

@sethreidnz
Copy link

sethreidnz commented Aug 7, 2023

Clear and concise description of the problem

There are a few threads where people are describing having issues with hanging process in Vitest. I'm convinced that the vast majority of the time it is because of some sort of issue with a tests and not Vitest, however in some cases when one is moving a large codebase from another testing framework such as Jest where this issue did not surface due to different archectecture it can be a major blocker to adoption. I have this issue now where I am considering migrating back to Jest after a lot of work to move the codebase from CRA to vite wholesale because we simply cannot figure out what the cause is in our 10k+ tests.

This was not an issue in Jest.

Suggested solution

Allow for a flag such as --ignore-open-handles or something that allows users to opt into ignoring the open handles. I'm not sure if this is actually feasible but it would solve the problem for us as our tests are passing just fine, but they fail around 75% due to haning process which seems to be very hard to pin down.

Ideally this would happen as soon as all the tests are reported as passing as currently the test step in our CI/CD will eventually exit and fail with this issue, but it takes about 30mins before that happens. Is it technically feasible to go "oh look at the tests have passed, force exit process and exit with success code" or is there some technical reason why this is not possible?

I know ideally people would fix the root cause but sometimes this is not feasible and I would like to stay on Vitest but at this point it's becoming a terrible drain on our productivity and I don't know how much more time I can justify on trying to diagnose the issue, if we could simply ignore it (as I assume we were doing with Jest) then that would be preferable.

Alternative

No response

Additional context

No response

Validations

@AriPerkkio
Copy link
Member

Anything that helps preventing process hanging sounds good but what would this flag do in practice? Vitest already calls process.exit() but the process just won't terminate:

@sethreidnz
Copy link
Author

sethreidnz commented Aug 14, 2023

That is what I don't know and just thought I'd propose in case someone has any better ideas. I don't really understand the process of spawning processes and why the spawner can't force close them so thought I'd just suggest and see if anyone else had any ideas. It's strange that Vite can know the tests are passed but cannot stop the worker. But again I don't really have any understanding here so not suggesting that I have any ideas.

@sethreidnz
Copy link
Author

sethreidnz commented Aug 14, 2023

Having a quick look at Jest the way the do it is by going Worker.terminate() and they have an explicit flag for this:

https://github.com/jestjs/jest/blob/25a8785584c9d54a05887001ee7f498d489a5441/packages/jest-worker/src/workers/NodeThreadsWorker.ts#L283

https://jestjs.io/docs/cli#--forceexit

And they have unit tests too

https://github.com/jestjs/jest/blob/main/e2e/__tests__/forceExit.test.ts

@AriPerkkio
Copy link
Member

AriPerkkio commented Aug 14, 2023

Jest doesn't use that src/workers/NodeThreadsWorker.ts by default. It has to be manually enabled by https://jestjs.io/docs/configuration#workerthreads. Are you sure your test cases don't get stuck when that flag is enabled?

There are cases where NodeJS worker_threads get stuck without Vitest. I would expect Jest to get stuck too. #3077 (comment)

Vitest also calls the worker.terminate() by default:

https://github.com/tinylibs/tinypool/blob/56f8219892e0357a419fabc677a2d9100377b315/src/index.ts#L473-L489

@sethreidnz
Copy link
Author

Yeah fair enough that's probably true. I'm just brainstorming on ways to make it so that someone can come from Jest where the hanging issue doesn't occur by default. To Vitest where hanging is an issue. Perhaps that is impossible I'm not sure.

@AriPerkkio
Copy link
Member

New ideas to solve hanging process issues are always welcome but unfortunately this one doesn't really offer any solutions.

One thing that could help is to provide a reproduction case that makes Vitest hang. As far as I know, we have not received any reproduction cases that we could look into. The #2008 has plenty of discussion but no reproduction cases.
In #3077 there was a clear reproduction case and root cause was identified to be NodeJS issue related to compatibility with native fetch and worker_threads.

@sethreidnz
Copy link
Author

Thanks @AriPerkkio I agree, my understanding was not complete enough! I will close this comment unless I have something more actionable for you.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants