feat: add status tracking to RampsController ResourceState#8116
Open
feat: add status tracking to RampsController ResourceState#8116
Conversation
Add explicit status field to ResourceState to distinguish between uninitialized and empty-fetched states. The default idle/loading/success/error statuses are now tracked per resource and set explicitly via #setResourceStatus during fetch cycles in executeRequest. This eliminates ambiguity when determining if a resource has been fetched vs is still in its initial state, fixing false positive "token unavailable" errors in mobile when payment methods are actually still loading.
7 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
AxelGes
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Mar 4, 2026
…ble errors Replace local ref-based loading tracking with paymentMethodsStatus from RampsController. The controller now exposes explicit 'idle'/'loading'/'success'/'error' status per resource, eliminating the need for component-level workarounds. Changes: - Expose status from useRampsPaymentMethods and thread through useRampsController - Replace hasSeenLoadingRef/prevProviderIdRef/prevSettledAssetIdRef with direct paymentMethodsStatus === RequestStatus.SUCCESS check - Add migration 124 to backfill status: 'idle' on persisted RampsController state - Make selectors tolerant of missing status field for backward compatibility - Update all tests to mock paymentMethodsStatus Depends on: MetaMask/core#8116
- Add missing status field reset in resetResource function to fix bugbot issue - Update test helpers (createResourceState, createDefaultResourceState) to include status field - Add createMockProvider helper to generate complete Provider objects with all required fields - Fix test mocks to use proper Provider type instead of partial objects - Update changelog with new status field feature
- Extend #updateResourceField to accept 'status' field, removing the duplicate update logic in #setResourceStatus - Move SUCCESS/ERROR status transitions to the finally block behind the same ref-count guard (next === 0) used by isLoading, so status and isLoading never disagree during concurrent requests
… branch coverage BuyWidget.url is typed as string (non-optional), so the ?? null was dead code that prevented 100% branch coverage.
…idle inconsistency When isResultCurrent() returns false, terminalStatus was never assigned, leaving status stuck at 'loading' while isLoading was set back to false. Fall back to RequestStatus.IDLE so the two fields always stay in sync.
…nsistent states Replace separate #setResourceLoading and #setResourceStatus calls with a single #setResourceLoadingAndStatus that writes both fields inside one this.update() call, so subscribers never observe a snapshot where isLoading and status disagree.
Contributor
Author
|
@metamaskbot release-preview |
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. |
amitabh94
reviewed
Mar 5, 2026
…tructor Persisted resource states (countries, providers, tokens) pre-dating the status field would come back from storage without it, leaving status undefined and defeating the idle/loading/success/error distinction. Deep-merge each persisted resource state with defaults in the constructor so that any missing fields are backfilled, while explicit values (including null, for the existing error-path test) are preserved.
The test title said "returns null" but the assertion checked for an empty string. The ?? operator does not coalesce empty strings, so the function correctly returns '' — the title was wrong, not the assertion.
…buyQuote, kycRequirement) The status field was only wired for top-level resources via executeRequest but native provider resources directly mutated isLoading/error without ever updating status, leaving it permanently 'idle'. Also backfill native provider and paymentMethods resource states in the constructor for persisted state migration.
amitabh94
previously approved these changes
Mar 5, 2026
…s to idle When multiple concurrent requests share a resourceType but different cache keys, the last-to-finish request now correctly preserves any terminal status (success/error) recorded by earlier current results rather than falling back to IDLE. Introduces #resourceTerminalStatus map to track the best status across all in-flight requests for a resource, cleared after the final request commits the status to state.
amitabh94
reviewed
Mar 5, 2026
…atus Persisted-state migration is a consumer responsibility (handled by migration 125 in metamask-mobile). The controller has no migration runner and should not paper over missing fields — that's the wrong layer for this concern.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
… region change Without this fix, a successful request recording SUCCESS into #resourceTerminalStatus before a region change aborts concurrent in-flight requests would cause that stale SUCCESS to persist. The next request batch for the same resource would then inherit it, preventing ERROR from being recorded even when the new request failed.
amitabh94
reviewed
Mar 6, 2026
| if (next === 0) { | ||
| this.#pendingResourceCount.delete(resourceType); | ||
| this.#setResourceLoading(resourceType, false); | ||
| // Use the best terminal status recorded by any current result across |
Contributor
There was a problem hiding this comment.
I know cursor commented on this but I wanted to understand if there would actually be a case where there will be concurrent requests and we need this ? My concern is that we may be over engineering for this case here.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
This PR adds explicit status tracking to the
ResourceStatetype in@metamask/ramps-controller. Previously, the controller used onlyisLoadinganderrorfields, making it impossible to distinguish between:This ambiguity caused false positive "token unavailable" errors in MetaMask Mobile when payment methods were still loading.
Changes
RampsController.ts
status: \${RequestStatus}`field toResourceState` typecreateDefaultResourceStateto initialize withstatus: RequestStatus.IDLE#setResourceStatusmethod to update resource statusexecuteRequest:loadingwhen fetch startssuccesswhen fetch completes successfullyerrorwhen fetch failsThe status field uses the existing
RequestStatusenum fromRequestCache.ts('idle' | 'loading' | 'success' | 'error'), already exported in the package's public API.References
Checklist
Note
Medium Risk
Moderate risk because it changes the persisted/public controller state shape and refactors request lifecycle logic around concurrency/abort paths, which could affect UI state transitions if incorrect.
Overview
Adds a
statusfield toResourceState(initialized toidle) so consumers can distinguish never fetched from fetched successfully with empty data, and wires this through default state creation and resource resets.Updates
executeRequestto set resourcestatustoloadingon cache misses and to commitsuccess/errorwhen the last in-flight request completes, using a new per-resource terminal-status tracker to avoid stale concurrent requests regressing status (and clearing this tracker on region-change dependent-request aborts). Transak resource fetch helpers are updated to setloading/success/errorstatuses, and tests are expanded/updated (includinggetWidgetUrlreturningnullforundefinedURLs but preserving empty strings, plus new provider mocks for order polling tests).Written by Cursor Bugbot for commit ae2ff7b. This will update automatically on new commits. Configure here.