fix: force timeout after 30s token detection #7106
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
salimtb
changed the title
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
sahar-fehri
reviewed
Nov 12, 2025
| supportedNetworks, | ||
| }); | ||
|
|
||
| return await Promise.race([apiCallPromise, timeoutPromise]); |
Contributor
There was a problem hiding this comment.
Hmm can we reuse our existing utils? E.g.
async #attemptAccountAPIDetection(
chainsToDetectUsingAccountAPI: Hex[],
addressToDetect: string,
supportedNetworks: number[] | null,
) {
// from controller-utils
const result = await safelyExecuteWithTimeout(
async () => {
return this.#addDetectedTokensViaAPI({
chainIds: chainsToDetectUsingAccountAPI,
selectedAddress: addressToDetect,
supportedNetworks,
});
},
false,
ACCOUNTS_API_TIMEOUT_MS,
);
if (!result) {
return { result: 'failed' };
}
return result;
}
Contributor
Author
There was a problem hiding this comment.
ooh true , comment addressed here e5b43d0
Contributor
Author
|
@metamaskbot publish-preview |
Prithpal-Sooriya
approved these changes
Nov 12, 2025
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
4 tasks
salimtb
added a commit
that referenced
this pull request
Nov 14, 2025
…7155) ## Explanation ### Current State and Problem Previously, when the Accounts API failed or timed out in `TokenBalancesController`, the error was caught within `AccountsApiBalanceFetcher` and converted into balance entries with `success: false`. While this seemed like graceful error handling, it created a critical issue: 1. The `TokenBalancesController` would receive these `success: false` entries as valid results 2. It would mark those chains as "processed" and remove them from `remainingChains` 3. The RPC fallback fetcher would never attempt to fetch balances for those chains 4. **Result:** Users would get no balance data when the API failed, instead of falling back to RPC Additionally, there was no timeout protection, so slow or hanging API calls could delay balance updates indefinitely. ### Solution This PR enables proper RPC fallback by: 1. **Adding timeout protection:** Wraps API calls with `safelyExecuteWithTimeout` (30-second timeout) to prevent hanging requests 2. **Propagating errors:** Removes the try-catch block that was swallowing API errors, allowing them to propagate to `TokenBalancesController` 3. **Leveraging existing fallback logic:** `TokenBalancesController` already has proper error handling (lines 687-692) that catches errors and moves to the next fetcher 4. **Simplifying the flow:** Removed the `apiError` flag and related logic for generating `success: false` entries When the API fails or times out: - `safelyExecuteWithTimeout` returns `undefined` - We detect this and throw an error: `'Accounts API request timed out or failed'` - `TokenBalancesController` catches it and automatically falls back to RPC - RPC fetcher handles **both** native balances and staked balances ### Key Changes **`api-balance-fetcher.ts`:** - Import and use `safelyExecuteWithTimeout` with 30-second timeout - Remove try-catch that prevented error propagation - Remove `apiError` flag logic - Remove code that generated `success: false` entries when API failed - Throw error when `apiResponse` is `undefined` **`api-balance-fetcher.test.ts`:** - Updated 2 tests to expect error propagation instead of graceful handling - Tests now verify that errors are thrown with message: `'Accounts API request timed out or failed'` ### Non-Obvious Changes The original try-catch was attempting to fetch staked balances even when the API failed. This is no longer necessary because: - When errors propagate to `TokenBalancesController`, it falls back to RPC - The RPC fetcher handles **all** balance types (native + staked + tokens) - This results in a simpler, more reliable flow ## References - Related to #7106 (similar timeout protection added to `TokenDetectionController`) - Part of ongoing improvements to balance fetching reliability and fallback mechanisms ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes (N/A - no breaking changes) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds 30s timeout and error propagation in `AccountsApiBalanceFetcher` so `TokenBalancesController` falls back to RPC on Accounts API failures/timeouts; updates tests and changelog. > > - **Balances fetching (`AccountsApiBalanceFetcher`)**: > - Add 30s timeout via `safelyExecuteWithTimeout` for Accounts API requests. > - Remove internal try/catch and `apiError` logic; propagate failures. > - Throw `'Accounts API request timed out or failed'` when API fails/returns undefined to trigger RPC fallback. > - Always add zero native entries only when API succeeds but omits them; remove error-entry generation. > - Preserve handling of `unprocessedNetworks` -> `unprocessedChainIds`. > - **Tests** (`api-balance-fetcher.test.ts`): > - Update expectations to assert thrown error on API failure (for RPC fallback). > - Add coverage for batching accumulation, integer-only balances, and timeout-failure path. > - **Changelog**: > - Document RPC fallback enablement, 30s timeout, and related fixes in `TokenBalancesController`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0f83296. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
What is the current state and why does it need to change?
Currently, when
TokenDetectionControlleruses the Accounts API to detect tokens, API calls can hang indefinitely if the API is slow, unresponsive, or experiencing network issues. This causes the entire token detection process to freeze without any fallback mechanism, resulting in a poor user experience where tokens are never detected.What is the solution and how does it work?
This PR adds 30-second timeout protection for Accounts API calls in
TokenDetectionController:Added
ACCOUNTS_API_TIMEOUT_MSconstant (30000ms) to define the timeout thresholdWrapped Accounts API calls with
Promise.race()between the actual API call and a timeout promiseWhen timeout occurs, the promise rejects and is caught, triggering an automatic fallback to RPC-based token detection
Properly cleans up the timeout in a
finallyblock to prevent memory leaksIncludes error logging for debugging timeout and failure events
The timeout mechanism ensures that:
If the API responds within 30 seconds, detection proceeds normally via the API
If the API takes longer than 30 seconds, the timeout fires and RPC detection takes over
Users always get token detection results, either via API or RPC fallback
Changes that might not be obvious
The timeout is implemented in the
#attemptAccountAPIDetectionprivate method, which wraps the existing#addDetectedTokensViaAPIcall. This ensures the timeout protection applies to all Accounts API token detection flows without requiring changes to the core detection logic.References
Improves reliability of token detection by preventing indefinite hangs
Ensures users always get token detection results through automatic RPC fallback
Related to ongoing work to improve Accounts API reliability and user experience
Checklist
I've updated the test suite for new or updated code as appropriate
Added comprehensive timeout test using fake timers to simulate 30-second timeout scenario
Test verifies that API is called, timeout triggers, RPC fallback occurs, and tokens are successfully added
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
CHANGELOG.mdwith timeout protection fixI've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
Technical Implementation Details
Key Files Changed
TokenDetectionController.tsACCOUNTS_API_TIMEOUT_MSconstant (30000ms)#attemptAccountAPIDetectionmethod to implement timeout protectionPromise.race()to race between API call and timeout promisefinallyblock to prevent memory leaksTokenDetectionController.test.ts'should timeout and fallback to RPC when Accounts API call takes longer than 30 seconds'sinon.useFakeTimers()to simulate time advancementCode Flow
Testing Strategy
The test uses fake timers to simulate the 30-second timeout without actually waiting 30 seconds:
Impact
User Experience
Performance
Reliability
Note
Adds a 30s timeout to Accounts API token detection with RPC fallback, and refactors balance fetchers to return
unprocessedChainIdsso controllers can retry unsupported chains via RPC.TokenDetectionController):ACCOUNTS_API_TIMEOUT_MS(30s) and wrap API calls withsafelyExecuteWithTimeout; on timeout/failure, fall back to RPC.unprocessedNetworksand add those chains to RPC detection.fetch()to return{ balances, unprocessedChainIds }inAccountsApiBalanceFetcherandRpcBalanceFetcher.unprocessedNetworks(decimal) to hexunprocessedChainIds; aggregate across batches.AccountTrackerRPC fetcher wrapper, filter out staked entries whenincludeStakedAssetsis false while preservingunprocessedChainIds.AccountTrackerControllerandTokenBalancesControllerto consume new fetcher result shape and re-queueunprocessedChainIdsfor fallback fetchers.CurrencyRateControllerresponses.CHANGELOG.mdto reflect timeout protection and unprocessed network handling.Written by Cursor Bugbot for commit 7a813ba. This will update automatically on new commits. Configure here.