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: Gracefully handle bad responses from net_version calls to RPC endpoint when getting Provider Network State #27509

Merged
merged 10 commits into from
Nov 21, 2024

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented Sep 30, 2024

Description

Currently some users are seeing Error: Cannot parse as a valid network ID: 'undefined' errors when connected to RPC endpoints that fail to provide a value from net_version calls that can be parsed into a decimal string. This PR makes our internally handling of this case more flexibly by using null as the network version value sent to the inpage provider when receiving an unexpected net_version call value.

Open in GitHub Codespaces

Related issues

Fixes: #27487

Manual testing steps

  1. Open extension background
  2. Intercept a request for net_version (you can use a proxy like Burpsuite) and make it return a bad value
  3. There should be no error in extension background related to this
  4. Open a dapp
  5. Open the dapp developer console
  6. Expect to see an error like JsonRpcError: MetaMask: Disconnected from chain. Attempting to connect.
  7. Test that window.ethereum.request works as expected despite this

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@jiexi jiexi marked this pull request as ready for review September 30, 2024 18:07
@jiexi jiexi requested a review from a team as a code owner September 30, 2024 18:07
@metamaskbot
Copy link
Collaborator

Builds ready [3d4a7e2]
Page Load Metrics (1680 ± 80 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15432328168216278
domContentLoaded14672316166416680
load14752336168016880
domInteractive24202584723
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 11 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -41 Bytes (-0.00%)

Copy link

sonarqubecloud bot commented Oct 4, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [5e7f833]
Page Load Metrics (1846 ± 128 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint154828791845277133
domContentLoaded154128131829267128
load154828201846266128
domInteractive2696542211
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 11 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -41 Bytes (-0.00%)

Copy link
Contributor

@hjetpoluru hjetpoluru left a comment

Choose a reason for hiding this comment

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

LGTM

@legobeat legobeat requested a review from a team October 21, 2024 19:00
@metamaskbot
Copy link
Collaborator

Builds ready [5165b03]
Page Load Metrics (1768 ± 90 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15712335176817283
domContentLoaded15622175173115273
load15702415176818790
domInteractive20172523617
backgroundConnect9230374924
firstReactRender42194903818
getState593292813
initialActions01000
loadScripts11691649128912560
setupStore1096282411
uiStartup172133212007327157
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 11 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -41 Bytes (-0.00%)

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@jiexi jiexi enabled auto-merge November 19, 2024 18:34
@jiexi jiexi added this pull request to the merge queue Nov 20, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [19060cc]
Page Load Metrics (2062 ± 108 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint40724561978423203
domContentLoaded160123752029215103
load161124642062224108
domInteractive17178503316
backgroundConnect1088332612
firstReactRender66221994019
getState51202963818
initialActions01000
loadScripts11631835153218488
setupStore78215199
uiStartup192633002407305146
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 11 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -41 Bytes (-0.00%)

Merged via the queue into develop with commit 1975c85 Nov 21, 2024
75 checks passed
@jiexi jiexi deleted the jl/mme-27487/fix-net_version-bad-response-error branch November 21, 2024 00:06
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2024
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-wallet-api-platform
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Error: Cannot parse as a valid network ID: 'undefined'
5 participants