Skip to content

Commit

Permalink
fix: Intermittent Display Issue of Fiat Currency on Main Wallet View (#…
Browse files Browse the repository at this point in the history
…11087)

## **Description**

This bug is difficult to reproduce organically, but we suspect that the
root cause is that `fetchExchangeRate` function to crypto-compare could
sometimes be down, causing the function to throw an error prematurely.
We were able to simulate this locally, and it caused the zero balance to
occur.

In `CurrencyController` we `updateExchangeRate` in a finally block (so,
even when there is an error in the fetchExchangeRates call). In this
case, the Controller will returns nullish values, causing the balance to
zero out.

This patch only updates state in the `try` block, after a successful
response. We are maintaining existing error handling in the `catch`
statement, and keeping the `releaseLock` in the finally to handle
concurrent requests gracefully.

## **Related issues**

Fixes: #11035

## **Manual testing steps**

1. Run app normally, to get initial balance
2. Throw error in `fetchExchangeRate` function
3. Balance should not zero out like it did before this patch.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/user-attachments/assets/ac0fa1c2-9f4d-4e42-85f4-9b5edb8fc753

### **After**


https://github.com/user-attachments/assets/62c3fc40-9dfc-4573-97e5-6c2a1fabcec8

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
  • Loading branch information
gambinish authored Sep 6, 2024
1 parent 6501cbb commit 387b9e8
Showing 1 changed file with 53 additions and 0 deletions.
53 changes: 53 additions & 0 deletions patches/@metamask+assets-controllers+30.0.0.patch
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,59 @@ index 0dc70ec..461a210 100644
}
};
return acc;
diff --git a/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js b/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js
index 45254ad..83634ab 100644
--- a/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js
+++ b/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js
@@ -87,6 +87,7 @@ var CurrencyRateController = class extends _pollingcontroller.StaticIntervalPoll
const nativeCurrencyForExchangeRate = Object.values(
_controllerutils.TESTNET_TICKER_SYMBOLS
).includes(nativeCurrency) ? _controllerutils.FALL_BACK_VS_CURRENCY : nativeCurrency;
+ let shouldUpdateState = true;
try {
if (currentCurrency && nativeCurrency && // if either currency is an empty string we can skip the comparison
// because it will result in an error from the api and ultimately
@@ -103,23 +104,27 @@ var CurrencyRateController = class extends _pollingcontroller.StaticIntervalPoll
}
} catch (error) {
if (!(error instanceof Error && error.message.includes("market does not exist for this coin pair"))) {
+ // Don't update state on transient / unexpected errors
+ shouldUpdateState = false;
throw error;
}
} finally {
try {
- this.update(() => {
- return {
- currencyRates: {
- ...currencyRates,
- [nativeCurrency]: {
- conversionDate,
- conversionRate,
- usdConversionRate
- }
- },
- currentCurrency
- };
- });
+ if (shouldUpdateState) {
+ this.update(() => {
+ return {
+ currencyRates: {
+ ...currencyRates,
+ [nativeCurrency]: {
+ conversionDate,
+ conversionRate,
+ usdConversionRate
+ }
+ },
+ currentCurrency
+ };
+ });
+ }
} finally {
releaseLock();
}
diff --git a/node_modules/@metamask/assets-controllers/dist/chunk-FMZML3V5.js b/node_modules/@metamask/assets-controllers/dist/chunk-FMZML3V5.js
index ee6155c..06a3a04 100644
--- a/node_modules/@metamask/assets-controllers/dist/chunk-FMZML3V5.js
Expand Down

0 comments on commit 387b9e8

Please sign in to comment.