Conversation
src/components/charts/SwipeChart.tsx
Outdated
| if ('status' in marketChartRange) { | ||
| if (marketChartRange.status.error_code === 429) { | ||
| // Rate limit error, use our API key as a fallback | ||
| if (!fetchUrl.includes('x_cg_pro_api_key')) { |
There was a problem hiding this comment.
Can you remove the dummy coingecko key from the config cleaner and add a check for it's existence here? Passing a dummy value to the api will produce a 10002 error which is unhandled here and produces an error (an uglier error message with this commit's change). Instead, if the key doesn't exist we should just try the fetch again without an api key.
| const { countryCode } = this.state | ||
|
|
||
| const showStaking = this.isStakingAvailable() | ||
| const hideStaking = !isStakingSupported(this.props.wallet.currencyInfo.pluginId, this.props.currencyCode) |
There was a problem hiding this comment.
There's a remaining corner case that any asset with the currency code FIO (like a custom token) will show the earn button and navigate to actual FIO staking. The FIO plugin special currency info has isStakingSupported: true so that special case in the helper function can be removed.
src/components/charts/SwipeChart.tsx
Outdated
| if (!fetchUrl.includes('x_cg_pro_api_key')) { | ||
| if (!fetchUrl.includes('x_cg_pro_api_key') && ENV.COINGECKO_API_KEY !== '') { | ||
| fetchUrl = `${COINGECKO_URL_PRO}${fetchPath}&x_cg_pro_api_key=${ENV.COINGECKO_API_KEY}` | ||
| continue |
There was a problem hiding this comment.
The continue should be moved outside of the apikey block so that rate-limited requests without keys can be retried as well.
There was a problem hiding this comment.
The continue retries the request. Why should it be outside of the retry path?
There was a problem hiding this comment.
It needs to be in the 429 block but outside of the apikey block
|
/rebase |
Unknown errors shouldn't be caught by sub-routines, but rather should be handled at the top of the call-stack. Errors were not properly logged or shown to the user with this incorrect try-catch usage. Without it, the user is presented with errors correctly because it uses the right handler.
Fixes incorrect logic about when to show staking button caused by recent refactor.
The ActivityIndicator in the earn button was incorrectly centered. The styles were inappropriate altogether.
Do or do not, there is not try-catch – Yoda Failure is not an option – Ed Harris
samholmes
force-pushed
the
sam/earn-staking-button-bug
branch
from
2f34dc7 to
5a23422
Compare
2 participants
This include the fixes for the Earn button, but it also includes some fixes to the error handling for coingecko API queries for the chart data because those were immediately apparent while fixing the original intent.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: