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: 29144 network switch failure handling #29295

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

Conversation

gambinish
Copy link
Contributor

@gambinish gambinish commented Dec 17, 2024

Description

When a user is on the Asset page or Token Details page, and they try to send/swap a token on a network that isn't the globally selected network, we used to throw an error to stop execution and prevent a user from sending a token on the wrong network.

Instead of not handling that thrown error, this PR introduces some code to handle that case more gracefully. Rather than throwing an unhandled error, we instead pop a toast if there is an error in the setCorrectChain function, and make sure that we do not proceed with navigation if the failure exists.

Open in GitHub Codespaces

Related issues

Fixes: #29144

Manual testing steps

  1. throw new Error('test error') in try statement of setCorrectChain function on coin-buttons.tsx
  2. Try to send/swap token on a network that isn't the globally selected network
  3. Validate that toast pops, is dismissible, and stops the user from navigating to the incorrect chain

Screenshots/Recordings

Before

Screen.Recording.2024-12-17.at.1.09.48.PM.mov

After

Screen.Recording.2024-12-17.at.1.06.42.PM.mov

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.

@gambinish gambinish changed the title Fix/29144 network switch failure handling fix: 29144 network switch failure handling Dec 17, 2024
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.

@gambinish gambinish marked this pull request as ready for review December 17, 2024 20:55
@metamaskbot
Copy link
Collaborator

Builds ready [bb979b6]
Page Load Metrics (1559 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint46618111507266128
domContentLoaded13791775153211354
load14131802155911555
domInteractive24130382713
backgroundConnect97133189
firstReactRender1574342311
getState5471194
initialActions01000
loadScripts1025134811449144
setupStore66112147
uiStartup15952127179116177
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.71 KiB (0.02%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [a469076]
Page Load Metrics (1684 ± 97 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14172420168720498
domContentLoaded14082413165620297
load14162418168420397
domInteractive248639189
backgroundConnect97235199
firstReactRender1693422713
getState55817178
initialActions01000
loadScripts1014140911998842
setupStore65112115
uiStartup164426681929261125
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.71 KiB (0.02%)
  • common: 61 Bytes (0.00%)

Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya left a comment

Choose a reason for hiding this comment

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

Approved with some comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Sentry] Error: Had a problem changing networks!
3 participants