Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix: after provider.disconnect() is called, Ganache should stop serving requests #3433

Merged
merged 14 commits into from
Aug 19, 2022

Conversation

jeffsmale90
Copy link
Contributor

@jeffsmale90 jeffsmale90 commented Jul 27, 2022

Previously, when provider.disconnect() was called, a lot of the internals of Ganache would be stopped, but just enough was kept alive to serve requests. This could stop consumers from shutting down cleanly.

Now all pending requests, and any new requests will be rejected with a helpful error.

For example, before this change, the following would successfully return the block number:

const provider = Ganache.provider();
await provider.disconnect();
const blockNumber = await provider.send("eth_blockNumber");

But now:

Error: Cannot process request, Ganache is disconnected.

Caveat: any request that has started to process, but not yet complete, may fail with an unexpected error. See #3499 for details.

Fixes #3293

@jeffsmale90 jeffsmale90 changed the title fix: once provider.disconnect() is called, Ganache should stop serving requests fix: after provider.disconnect() is called, Ganache should stop serving requests Jul 27, 2022
Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jeffsmale90. I spent a moment to marvel at the fix and left some minor nits.

src/packages/utils/src/utils/request-coordinator.ts Outdated Show resolved Hide resolved
src/packages/utils/src/utils/request-coordinator.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool stuff!

src/packages/utils/src/utils/request-coordinator.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/provider.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/provider.ts Outdated Show resolved Hide resolved
src/packages/utils/src/utils/executor.ts Show resolved Hide resolved
@jeffsmale90 jeffsmale90 force-pushed the fix/provider_disconnect branch 3 times, most recently from 3d555b9 to 2b464fd Compare August 5, 2022 02:45
@jeffsmale90
Copy link
Contributor Author

I've narrowed the scope of this PR as suggested by @davidmurdoch. It now just stops processing requests, and rejects pending tasks.

It makes a "best effort" attempt to resolve in order, but will fail in most cases, because we do not wait for inflight requests to resolve before shutting down the blockchain. See #3499 to cover those changes.

Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question and a comment for you!

src/packages/utils/tests/request-coordinator.test.ts Outdated Show resolved Hide resolved
src/packages/utils/src/utils/request-coordinator.ts Outdated Show resolved Hide resolved
…ConnectorLoader to _not_ maintain a reference to requestCoordinator.resume(), and test obscure edge case.
Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐⭐⭐⭐⭐

Would review again!

@jeffsmale90 jeffsmale90 merged commit 32a7814 into develop Aug 19, 2022
@jeffsmale90 jeffsmale90 deleted the fix/provider_disconnect branch August 19, 2022 04:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ganache Provider does not shut down in eager mode
4 participants