feat: add status tracking to RampsController ResourceState#8116
Closed
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
Remove the cross-request terminal status tracking map and its associated #recordResourceTerminalStatus method in favour of a simple let variable scoped to each executeRequest closure. The last concurrent request to finish now wins (last-write-wins), which is simpler and sufficient given the AbortController already cancels stale requests.
georgeweiler
added a commit
that referenced
this pull request
Mar 10, 2026
This was referenced Mar 10, 2026
auto-merge was automatically disabled
March 10, 2026 16:40
Pull request was closed
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
Adds a new
statusfield to the persisted controller state and changesexecuteRequestconcurrency/abort finalization semantics, which can affect UI logic that depends on resource lifecycle and empty/initial states.Overview
Adds explicit request lifecycle status to ramp resources.
ResourceStatenow includesstatus(idle/loading/success/error), defaults/migrations fill it in, and dependent-resource resets copy it so consumers can distinguish never fetched from fetched empty.Updates request execution and native Transak flows to set status.
executeRequestnow updates resourcestatusalongsideisLoading(including last-write-wins behavior for concurrent requests and better handling around stale/aborted requests), and Transak helper methods (transakGetUserDetails/transakGetBuyQuote/transakGetKycRequirement) setstatuson load/success/error. Tests and changelog are updated, with additional coverage for concurrency edge cases andgetWidgetUrlnull/undefined/empty-url handling expectations.Written by Cursor Bugbot for commit af741b2. This will update automatically on new commits. Configure here.