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

node does not exit #369

Closed
electriquo opened this issue May 4, 2020 · 10 comments
Closed

node does not exit #369

electriquo opened this issue May 4, 2020 · 10 comments

Comments

@electriquo
Copy link

electriquo commented May 4, 2020

i use prom-client version 12.0.0.

when i run jest --detectOpenHandles i get

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  ELDHISTOGRAM

      3 | const client = require("prom-client");
      4 | const prefix = "name_";
    > 5 | client.collectDefaultMetrics({ prefix });

following not returning an interval, i saw the docs update. also looked at the tests and saw that indeed clearInterval() is called.

could one please shed the light on how should prom-client be closeed properly?

@zbjornson
Copy link
Collaborator

I think we need to be disabling the event loop delay histogram in an exit handler...

@sam-github
Copy link
Contributor

It should be unrefed, best to avoid exit handlers, except it doesn't appear to have a documented unref. It might be there anyway, I'll go look.

subject should be fixed, "exists" was meant to be "exit", I think.

@zbjornson zbjornson changed the title node does not exist node does not exit May 6, 2020
@sam-github
Copy link
Contributor

This is a false-positive on the part of jest, I can't reproduce a scenario where node does not exit, either by calling the default collector, or by call perf_hooks.

@y0y0z can you provide a reproduction of failure to exit?

This may be a bug that should be reported to jest.

@sam-github
Copy link
Contributor

Also, note that while the tests are calling clearInterval(), this is either legacy code to be deleted, or possibly, a check of backward compatibility of the API. The so-called interval in interval = collectDefaultMetrics() is now always undefined (no interval timer is used anymore).

@electriquo
Copy link
Author

Also, note that while the tests are calling clearInterval(), this is either legacy code to be deleted, or possibly, a check of backward compatibility of the API. The so-called interval in interval = collectDefaultMetrics() is now always undefined (no interval timer is used anymore).

@sam-github: #342

indeed the tests are for the old implementation and does not coincide with the issue #342.

currently, i do not have a repository to reproduce the issue, will try my best to share something later on. meanwhile, i think revising the current test and fix them to reflect the changes, might reveal the issue.

@sam-github
Copy link
Contributor

Removing those lines from the tests doesn't change jest's opinion about whether prom-client is leaking handles. If there is no problem with prom-client (and I can't find one), then the issue should be closed and opened against jest.

@SimenB
Copy link
Collaborator

SimenB commented May 11, 2020

Jest should not count ELDHISTOGRAM or PerformanceObserver. They don't keep the process open, so Jest shouldn't care about them.

Wanna send a PR? Code is here: https://github.com/facebook/jest/blob/0e0eeed9d794fe0d6d7e84f555d9684f9e944221/packages/jest-core/src/collectHandles.ts#L55-L57

If you don't have time to send a PR, could you open up an issue?

(I'm one of the maintainers of Jest).

@electriquo
Copy link
Author

@SimenB: thank you. i opened an issue.

@SimenB
Copy link
Collaborator

SimenB commented May 11, 2020

Thanks!

@micaelmbagira
Copy link

micaelmbagira commented Feb 26, 2021

@SimenB PR is open jestjs/jest#11123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants