fix(ramps-controller): expose circuit breaker error key#8596
Open
saustrie-consensys wants to merge 7 commits intomainfrom
Open
fix(ramps-controller): expose circuit breaker error key#8596saustrie-consensys wants to merge 7 commits intomainfrom
saustrie-consensys wants to merge 7 commits intomainfrom
Conversation
saustrie-consensys
had a problem deploying
to
default-branch
GitHub Actions
Failure
saustrie-consensys
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
10 tasks
Contributor
Author
|
Follow-up pushed in The main change in thinking here is that the circuit breaker is an internal service-policy signal, not provider copy. This follow-up tightens that contract in two ways:
That keeps the mobile side decoupled from |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Reviewed by Cursor Bugbot for commit cbe9a86. Configure here.
Contributor
|
Can you please publish a preview of this PR and use that to create a mobile PR to see if anything breaks? |
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
Author
|
Published the preview and wired it into a dedicated draft mobile validation PR. Preview build used:
Mobile validation PR: A useful thing this surfaced immediately: switching mobile to the published preview introduced a second Local validation on that branch:
I kept this separate from the main mobile companion PR on purpose so the preview-consumer plumbing stays isolated and easy to drop once we’re done validating the packaged artifact. |
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
3 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
When repeated upstream failures trip the service-policy circuit breaker,
RampsControllerreceives aBrokenCircuitErrorfrom@metamask/controller-utils. Cockatiel's default message for that condition isExecution prevented because the circuit breaker is open, which is useful for logs/debugging but not something clients should localize by string-matching.This PR keeps
coreout of copy ownership by:RAMPS_ERROR_CODES.CIRCUIT_BREAKER_OPENerrorKeymetadata to classified request/resource failureserrorKeyso clients can localize themFollow-up after review tightened the contract in two ways:
error instanceof BrokenCircuitErrorfirst, with the previous message match retained only as a compatibility fallback for wrapped/plain errors that have lost the prototypeerrorKeyacross the broader set of policy-backed Transak wrappers, so native-flow callers receive consistent metadata instead of a mix of keyed and raw infrastructure errorsThis keeps the boundary clean:
coredecides what kind of failure this is, and clients decide which localized fallback to show. Coverage now includes the sharedexecuteRequestpath, request/resource state, selector behavior, and the Transak methods used by native flows that previously rethrew raw breaker errors.The companion mobile change now uses that boundary to show a dedicated circuit-breaker fallback instead of the previous generic quote error. The copy says
about 30 minutesbecause the service-policy cooldown defaults to 30 minutes, but the current ramps error contract does not carry a live remaining-duration value.Demo Videos
These two recordings were captured by wiring the local
corecheckout into the local mobile checkout and hard-throwing the sameBrokenCircuitErrorinside theRampsController.getQuotesfetcher on both branches.GitHub sanitizes
iframemarkup in PR bodies, so the demos are embedded with HTML<video>tags backed by GitHub-hosted MP4s instead.Before (
main)After (this core PR + mobile PR branch)
References
Testing
yarn workspace @metamask/ramps-controller testyarn workspace @metamask/ramps-controller messenger-action-types:checkyarn eslint packages/ramps-controller/src/RampsController.ts packages/ramps-controller/src/RampsController.test.ts packages/ramps-controller/src/RequestCache.ts packages/ramps-controller/src/selectors.ts packages/ramps-controller/src/selectors.test.ts packages/ramps-controller/src/index.ts packages/ramps-controller/src/rampsErrorCodes.tsyarn tsc --noEmit -p packages/ramps-controller/tsconfig.json(still fails in this checkout due existing referenced-build / monorepo type issues, includingTS6305build-artifact errors and unchanged pre-existingTransakService.tstype errors)yarn workspace @metamask/ramps-controller run jest --no-coverage src/RampsController.test.tsyarn eslint packages/ramps-controller/src/RampsController.ts packages/ramps-controller/src/RampsController.test.tsChecklist
Note
Medium Risk
Touches shared error-handling and state-shaping paths (requests/resources/selectors) and changes what gets thrown/returned on failures, which could affect downstream consumers that assume previous error shapes.
Overview
RampsControllernow classifies circuit-breaker failures and propagates a stableerrorKey(RAMPS_ERROR_CODES.CIRCUIT_BREAKER_OPEN) alongside the original error message, so clients can localize fallback copy without string-matching Cockatiel text.This threads
errorKeythrough cached request state (createErrorState/RequestState), resource state (ResourceState+ reset/clear paths), and request selectors (optionally returningerrorKey), and normalizes/rethrows errors fromexecuteRequestand multiple Transak helper methods to include the sameerrorKeyeven when the thrown value isn’t anError(string/object). Tests and changelog are updated accordingly, and an eslint suppression entry is removed forRampsController.ts.Reviewed by Cursor Bugbot for commit 7cef09c. Bugbot is set up for automated code reviews on this repo. Configure here.