-
Notifications
You must be signed in to change notification settings - Fork 287
Add more robust error handling to SwipeChart
#5955
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -286,49 +286,76 @@ export const SwipeChart: React.FC<Props> = props => { | |
| ) | ||
| // Start with the free base URL | ||
| let fetchUrl = `${COINGECKO_URL}${fetchPath}` | ||
| do { | ||
| try { | ||
| // Construct the dataset query | ||
| const response = await fetch(fetchUrl) | ||
| const result = await response.json() | ||
| const apiError = asMaybe(asCoinGeckoError)(result) | ||
| if (apiError != null) { | ||
| if (apiError.status.error_code === 429) { | ||
| // Rate limit error, use our API key as a fallback | ||
| 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}` | ||
| try { | ||
| do { | ||
| try { | ||
| const response = await fetch(fetchUrl) | ||
|
|
||
| // Handle non-OK responses before parsing JSON | ||
| if (!response.ok) { | ||
| if (response.status === 429) { | ||
| 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}` | ||
| } | ||
| // Wait 2 seconds before retrying. It typically takes 1 minute | ||
| // before rate limiting is relieved, so even 2 seconds is hasty. | ||
| await snooze(2000) | ||
| continue | ||
| } | ||
| // Wait 2 second before retrying. It typically takes 1 minute | ||
| // before rate limiting is relieved, so even 2 seconds is hasty. | ||
| await snooze(2000) | ||
| continue | ||
| throw new Error(`HTTP ${response.status}: ${response.statusText}`) | ||
| } | ||
| throw new Error( | ||
| `Failed to fetch market data: ${apiError.status.error_code} ${apiError.status.error_message}` | ||
| ) | ||
| } | ||
|
|
||
| const marketChartRange = asCoinGeckoMarketChartRange(result) | ||
| const rawChartData = marketChartRange.prices.map(pair => ({ | ||
| x: new Date(pair[0]), | ||
| y: pair[1] | ||
| })) | ||
| const reduced = reduceChartData(rawChartData, selectedTimespan) | ||
|
|
||
| setChartData(reduced) | ||
| cachedTimespanChartData.set(selectedTimespan, reduced) | ||
| setCachedChartData(cachedTimespanChartData) | ||
| } catch (e: unknown) { | ||
| console.error(JSON.stringify(e)) | ||
| setErrorMessage(lstrings.error_data_unavailable) | ||
| } finally { | ||
| setIsFetching(false) | ||
| } | ||
| break | ||
| } while (true) | ||
| // Parse JSON safely - use text() first to catch parse errors locally | ||
| const text = await response.text() | ||
| let result: unknown | ||
| try { | ||
| result = JSON.parse(text) | ||
| } catch { | ||
| throw new Error(`Invalid JSON response: ${text.slice(0, 100)}...`) | ||
| } | ||
|
|
||
| const apiError = asMaybe(asCoinGeckoError)(result) | ||
| if (apiError != null) { | ||
| if (apiError.status.error_code === 429) { | ||
| 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}` | ||
| } | ||
| // Wait 2 seconds before retrying. It typically takes 1 minute | ||
| // before rate limiting is relieved, so even 2 seconds is hasty. | ||
| await snooze(2000) | ||
| continue | ||
| } | ||
| throw new Error( | ||
| `Failed to fetch market data: ${apiError.status.error_code} ${apiError.status.error_message}` | ||
| ) | ||
| } | ||
|
|
||
| const marketChartRange = asCoinGeckoMarketChartRange(result) | ||
| const rawChartData = marketChartRange.prices.map(pair => ({ | ||
| x: new Date(pair[0]), | ||
| y: pair[1] | ||
| })) | ||
| const reduced = reduceChartData(rawChartData, selectedTimespan) | ||
|
|
||
| setChartData(reduced) | ||
| cachedTimespanChartData.set(selectedTimespan, reduced) | ||
| setCachedChartData(cachedTimespanChartData) | ||
| } catch (e: unknown) { | ||
| const message = e instanceof Error ? e.message : String(e) | ||
| console.error('SwipeChart fetch error:', message) | ||
| setErrorMessage(lstrings.error_data_unavailable) | ||
| } | ||
| break | ||
| } while (true) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unbounded retry loop on 429 can spin foreverHigh Severity The Additional Locations (1) |
||
| } finally { | ||
| setIsFetching(false) | ||
| } | ||
| }, | ||
| [selectedTimespan, isConnected, fetchAssetId, coingeckoFiat], | ||
| 'swipeChart' | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate 429 retry logic may drift
Low Severity
Two nearly identical blocks handle 429 retries — one for the HTTP status code (before JSON parsing) and one for the JSON body error code (after parsing). Both perform the same pro-URL fallback check,
snooze(2000), andcontinue. If future changes update one block (e.g., adding a retry counter or exponential backoff), the other could easily be missed, leading to inconsistent retry behavior.Additional Locations (1)
src/components/charts/SwipeChart.tsx#L321-L332