fix: Fix console errors upon switching networks #7278
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gudahtt
force-pushed
the
fix-unhandled-promise-rejection-errors
branch
from
86670ea
to
6f35519
Compare
On `main` two warnings were printed to the console repeatedly after switching networks. They were: * Error: Callback was already called. Possible Unhandled Promise Rejection * MaxListenersExceededWarning: Possible EventEmitter memory leak detected ... This started happening after PR #6872. If you look at network traffic, you could see that this lines up with extra network calls being made to the previous network as well (the `eth_getBlockByNumber` method specifically). These warnings were caused by a change made in `@metamask/network-controller@6.0.0`, which was to proxy the provider and block tracker. This introduced an incompatibility between the `NetworkController` and `web3-provider-engine`, which was still using the unproxied block tracker. `web3-provider-engine` adds an event listener on the `latest` event from the block tracker, which calls `eth_getBlockByNumber` with a retry. This listener was migrated to the new network after switching, but would make that RPC call on the old provider. The user effects of this are an increase in unnecessary network traffic, but is otherwise harmless. The `web3-provider-engine` package has been patched to make the block tracker event listener into a no-op after the provider has been stopped. This isn't an ideal solution as it still leaves the listener attached, which is a small memory leak. But it resolves the warnings and prevents the unnecessary network calls. We will be updating to `@metamask/network-controller` v9 soon which will resolve this problem permanently by removing `web3-provider-engine`. Fixes #7082
Gudahtt
force-pushed
the
fix-unhandled-promise-rejection-errors
branch
from
6f35519
to
6ca5cda
Compare
|
Passing E2E smoke test run: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/096eddd8-e5a5-4eee-8e68-b4e1e388670c |
Gudahtt
added
needs-dev-review
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #7278 +/- ##
=======================================
Coverage 34.43% 34.43%
=======================================
Files 1016 1016
Lines 27087 27087
Branches 2206 2206
=======================================
Hits 9327 9327
Misses 17267 17267
Partials 493 493 ☔ View full report in Codecov by Sentry. |
|
Kudos, SonarCloud Quality Gate passed!
|
Cal-L
approved these changes
Sep 20, 2023
Gudahtt
added
Spot Check on the Release Build
Cal-L
removed
Spot Check on the Release Build
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
On
maintwo warnings were printed to the console repeatedly after switching networks. They were:This started happening after PR #6872. If you look at network traffic, you could see that this lines up with extra network calls being made to the previous network as well (the
eth_getBlockByNumbermethod specifically).These warnings were caused by a change made in
@metamask/network-controller@6.0.0, which was to proxy the provider and block tracker. This introduced an incompatibility between theNetworkControllerandweb3-provider-engine, which was still using the unproxied block tracker.web3-provider-engineadds an event listener on thelatestevent from the block tracker, which callseth_getBlockByNumberwith a retry. This listener was migrated to the new network after switching, but would make that RPC call on the old provider.The user effects of this are an increase in unnecessary network traffic, but is otherwise harmless.
The
web3-provider-enginepackage has been patched to make the block tracker event listener into a no-op after the provider has been stopped. This isn't an ideal solution as it still leaves the listener attached, which is a small memory leak. But it resolves the warnings and prevents the unnecessary network calls. We will be updating to@metamask/network-controllerv9 soon which will resolve this problem permanently by removingweb3-provider-engine.Manual testing steps
yarn watchconsole output)Screenshots/Recordings
Before
https://recordit.co/SXZf4Vhnbz
After
https://recordit.co/5CIOzb4srH
Related issues
Fixes #7082
Pre-merge author checklist
Pre-merge reviewer checklist