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: handle send transaction API errors #26253

Merged
merged 5 commits into from
Jul 31, 2024
Merged

Conversation

BZahory
Copy link
Contributor

@BZahory BZahory commented Jul 31, 2024

Description

We are seeing a pair of sentry errors for a failing deconstruction error, Cannot destructure property 'id' of '(intermediate value)' as it is null:

This is because instead of returning an error when a transaction fails, we return null for the metadata.

This PR addresses the underlying issue by rethrowing the error for a failed transaction request, then adding UI treatments for the failed transaction.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Add an Error right before the sendTransaction background method in addTransactionAndRouteToConfirmationPage
  2. Go to the send flow
  3. Attempt a basic send
  4. Ensure that the page does not crash and the transactionErrored copy is shown

Screenshots/Recordings

Before

Screen.Recording.2024-07-31.at.12.26.52.PM.mov

After

Screen.Recording.2024-07-31.at.12.25.46.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.

@BZahory BZahory requested a review from a team as a code owner July 31, 2024 16:28
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.

@BZahory BZahory changed the title Handle send tx errors fix: handle send transaction API errors Jul 31, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [9d401b9]
Page Load Metrics (389 ± 299 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint603131206129
domContentLoaded8175353617
load401812389622299
domInteractive8175353617
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 408 Bytes (0.01%)
  • common: 8 Bytes (0.00%)

Copy link

codecov bot commented Jul 31, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 69.94%. Comparing base (2beba47) to head (8d99049).

Files Patch % Lines
ui/components/multichain/pages/send/send.js 7.69% 12 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #26253      +/-   ##
===========================================
- Coverage    69.95%   69.94%   -0.00%     
===========================================
  Files         1411     1411              
  Lines        49963    49972       +9     
  Branches     13800    13801       +1     
===========================================
+ Hits         34948    34952       +4     
- Misses       15015    15020       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ejwessel
ejwessel previously approved these changes Jul 31, 2024
micaelae
micaelae previously approved these changes Jul 31, 2024
@BZahory BZahory dismissed stale reviews from micaelae and ejwessel via 82ccf29 July 31, 2024 19:38
@BZahory BZahory force-pushed the handle-send-tx-errors branch from 82ccf29 to 8d99049 Compare July 31, 2024 19:41
Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [8d99049]
Page Load Metrics (71 ± 16 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint742251073115
domContentLoaded10111222110
load45194713316
domInteractive10111222110
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 410 Bytes (0.01%)
  • common: 8 Bytes (0.00%)

@BZahory BZahory merged commit e00863e into develop Jul 31, 2024
75 checks passed
@BZahory BZahory deleted the handle-send-tx-errors branch July 31, 2024 20:42
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
@metamaskbot metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Jul 31, 2024
@gauthierpetetin gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-bridge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants