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

web3.js Connection http agent hanging tests and cli utilities #25069

Closed
kklas opened this issue May 7, 2022 · 5 comments · Fixed by #29125
Closed

web3.js Connection http agent hanging tests and cli utilities #25069

kklas opened this issue May 7, 2022 · 5 comments · Fixed by #29125
Assignees
Labels
javascript Pull requests that update Javascript code web3.js Related to the JavaScript client

Comments

@kklas
Copy link
Contributor

kklas commented May 7, 2022

Problem

In the Connection class the AgentManager is used in order to pool http connections to the rpc server. Since the keepAlive option is used, the agent will hang until destroy() is called. To go around this there is a timeout which will destroy the agent 5 seconds after the last request.

But this still hangs the node process for 5 seconds which is an issue for CLI utilities and tests which use Connection. For example, it causes the following message when running jest tests (which I'm sure many have encoutered due to this):

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.

Proposed Solution

Either / or:

  1. add ability to pass a custom agent to be used via an option in the Connection constructor
  2. add a connection.close() method which will call agent.destroy() to clean up the connection gracefully

cc @steveluscher @jstarry

@steveluscher
Copy link
Contributor

Love it. Thanks for pointing this out.

@rabbygit
Copy link

rabbygit commented Nov 2, 2022

Is there any update about this issue?

@steveluscher
Copy link
Contributor

Hey! No update, no. Still worth doing though! We just have to do it thoughtfully. I haven't sat down to really think about whether making the Agent an explicit input is the right thing to do (breaking change) or whether to add a destructor (not effective unless people use it).

@DavidPesta
Copy link

Thanks for working on this. Looking forward to this being resolved. 👍

@steveluscher
Copy link
Contributor

K, I wasn't comfortable creating a whole bunch of new API surface area, so what I ended up doing was to:

  1. Let you override or disable the default agent via config on Connection
  2. Automatically disable the default agent when process.env.NODE_ENV is test, which should cover you in Jest.

#29125.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code web3.js Related to the JavaScript client
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants