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

Resolve test failure due to slow server shutdown #149

Closed
Wodann opened this issue Aug 1, 2023 · 3 comments
Closed

Resolve test failure due to slow server shutdown #149

Wodann opened this issue Aug 1, 2023 · 3 comments
Assignees
Milestone

Comments

@Wodann
Copy link
Member

Wodann commented Aug 1, 2023

Definition of Done
Ensure that the following tests succeeds:

  1) Provider logs
       JSON-RPC provider
         "after each" hook: Remove provider for "should log basic methods":
     Error: Closing the server took more than 1 second (5010ms), which can lead to incredibly slow tests. Please fix it.
      at Context.<anonymous> (test\internal\hardhat-network\helpers\useProvider.ts:155:15)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

source: https://github.com/NomicFoundation/hardhat/actions/runs/5730334578/job/15528932333

@agostbiro
Copy link
Member

agostbiro commented Oct 13, 2023

I looked into this and learned the following:

  1. I can't reproduce the failures locally.
  2. Running yarn build && DEBUG=hardhat:core:hardhat-network:jsonrpc yarn test -g "Provider logs JSON-RPC provider" in hardhat-core in the rethnet/main branch shows suspiciously long shutdown times for the HTTP server in the JsonRpcServer (900-1000ms).
    • It's probably due to the socket being kept open for some reason after the last request which prevents closing the server. This is a well-known problem with NodeJS.
    • I haven't managed to find what keeps the socket open in reasonable time.
    • I tried a hacky solution to force the server to close immediately. This resulted in the HTTP server shutting down immediately and tests were still passing.
    • Since the JsonRpcServer is exposed to users, I looked into finding a non-hacky solution by properly preventing keep alive/new connections when the server is shutting down and destroying sockets manually. There are several libraries that propose to do this, but none of them seem maintained. So we would have to write our own implementation which is non-trivial.
  3. Running the same command from 2. in main or hardhat-edr-merge shows that the HTTP server shuts down in <10ms.
    • I tried to find a diff that could account for this, but had no luck.

I'd propose that we wait until hardhat-edr-merge is merged and see if the problem persists after that before spending more time on it.

@Wodann
Copy link
Member Author

Wodann commented Oct 13, 2023

Thanks for the research, @agostbiro.

I feel out of my depth here, in NodeJS land. Do you have any insights on this, @alcuadrado or @fvictorio ?

@agostbiro
Copy link
Member

Running yarn build && DEBUG=hardhat:core:hardhat-network:jsonrpc yarn test -g "Provider logs JSON-RPC provider" in rethnet/main now yields server shutdown times of ~10ms as opposed to the prior ~1000ms before merging main into rethnet/main.

I also looked through the hardhat-core workflow failures in the latest commit in the last few PRs to rethnet/main and haven't seen this failure, so I think it's safe to say that this particular issue is resolved.

@Wodann Wodann modified the milestones: EDR v0.2.0, EDR v0.1.0 Oct 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants