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

fix: Network controller race condition after state update #5122

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bergeron
Copy link
Contributor

@bergeron bergeron commented Jan 9, 2025

Explanation

Fixes a race condition where:

  • You update a network with a new endpoint.
  • In the stateChange subscription callback, call getNetworkConfigurationByNetworkClientId on the new endpoint.
  • It returns undefined.

This occurs because the network client id -> network configuration map was being built after the state update, so it was not ready at the time of the stateChange event.

The fix involves moving this map building inside the state update, so that its built by the time stateChange fires. But also requires deep cloning the state before building the map from it. Otherwise we get Cannot perform 'get' on a proxy that has been revoked when referencing the ephemeral state lambda.

References

Changelog

@metamask/network-controller

  • FIXED: A race condition when calling getNetworkConfigurationByNetworkClientId in response to a stateChange event adding a new endpoint to a network.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@bergeron bergeron requested review from a team as code owners January 9, 2025 20:49
@bergeron bergeron requested a review from mcmire January 9, 2025 20:49
@bergeron bergeron requested a review from jiexi January 10, 2025 16:46
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

Successfully merging this pull request may close these issues.

1 participant