feat(ramp): headless buy fixes#30103
Open
saustrie-consensys wants to merge 11 commits into
Open
Conversation
Phase 9 (MMPay TPC dependency)
- New imperative `awaitOrderTerminalState(orderId, { timeoutMs?, pollIntervalMs?, walletAddress? })`
in `headless/orderTerminalState.ts` resolves with the `RampsOrder` once
its status reaches `Completed | Failed | Cancelled | IdExpired`. Drives
a redux subscription as the fast path and self-polls
`RampsController.getOrder` as the slow path so it is not coupled to
the unified order processor's `<FiatOrders />` mount lifecycle.
- New imperative `getOrder` and `refreshOrder(stringOrOrder)` siblings
in the same module — controllers can call them without going through
React. `useHeadlessBuy()` exposes thin passthroughs on the same names
for React consumers; `getOrderById` is preserved (deprecated) for
back-compat.
- Two typed errors with Hermes-safe `Object.setPrototypeOf` fixes:
`OrderTerminalStateTimeoutError` and `RefreshOrderUnresolvableError`.
- Playground gets an Order tracking panel after `onOrderCreated` —
surfaces `orderId`, current status (live from `getOrder`), and
Refresh / Await terminal state actions wired to the new API.
CHANGELOG entry: null
SonarQube flagged the playground for cognitive complexity (36 > 30) and three nested ternaries in the await-status badge rendering. Extracting `OrderTrackingPanel` as its own component pushes the complexity back under the threshold and clears the nested ternaries by routing color/text selection through `getAwaitBadgeColor` / `getAwaitBadgeText` helpers. No behaviour changes.
metamaskbotv2
Bot
added
the
team-money-movement
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
10 tasks
github-actions
Bot
added
the
pr-not-ready-for-e2e
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #30103 +/- ##
==========================================
+ Coverage 81.54% 81.75% +0.20%
==========================================
Files 5343 5388 +45
Lines 142128 143619 +1491
Branches 32411 32803 +392
==========================================
+ Hits 115899 117410 +1511
+ Misses 18299 18185 -114
- Partials 7930 8024 +94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Four fixes addressing complaints #2 (5 EUR provider rejection hang) and #3 (3-D Secure failure handling) from the May 12 testing thread. Complaint #1 (empty white screen) is scoped to the Phase 9.5 PR. - Fix #2: inspect `quotesResponse.error[]` in `getQuotes` and throw a structured `HeadlessBuyError` so the 5 EUR scenario surfaces to the consumer. Routes through the active session via `failSession` (Design B) when one exists so `onError` + `onClose` fire alongside the awaited rejection. - Fix #3.1: widen `onOrderCreated` to `(orderId, order)` and update both fire sites (aggregator widget in Checkout.tsx; shared `navigateToOrderProcessingCallback` in useTransakRouting.ts) plus its two callers. Lets MMPay read `order.walletAddress` without an extra `getOrder` round-trip. - Fix #3.2: load-bearing JSDoc on `onOrderCreated`, `awaitOrderTerminalState`, and `AwaitOrderTerminalStateOptions.timeoutMs` covering the asymmetric- callback contract (`onError` does not fire for created-then-failed orders), the creation-snapshot freshness caveat, the unbounded-timeout warning, and the recommended `{ walletAddress: order.walletAddress, timeoutMs: 5 * 60 * 1000 }` pattern. - Fix #3.3: add `AwaitOrderTerminalStatePrerequisitesError` (Hermes-safe via `Object.setPrototypeOf`) and a runtime pre-flight check in `awaitOrderTerminalState`. Rejects synchronously when the order isn't in redux AND `options.walletAddress` is not provided, converting a silent unbounded hang into an actionable error for the consumer's catch block. Slack thread: https://consensys.slack.com/archives/C0AK3NXRM7W/p1778577403631269 Test coverage: 255 tests across `app/components/UI/Ramp/{headless,hooks, Views/Checkout,Views/HeadlessPlayground}` — all green. New test cases cover provider-rejection classification (LIMIT_EXCEEDED vs QUOTE_FAILED, rate-limit false-positive guard, SDK code preference over message parsing), each fire site's new 2nd-arg passthrough, and the Fix #3.3 pre-flight reject + Hermes-safe cross-module `instanceof`. Out of scope (deferred): Fix #1 (transparent stack wrapper, Phase 9.5 PR's deferral list), `getUserLimits` swallow follow-up, `getProviderBuyLimit` belt-and-braces validation, Deposit feature (its identically-named `navigateToOrderProcessingCallback` is not wired to the headless session registry), and consumer-side TPC updates (TPC's own repo).
Four small clarifications from the post-implementation review of f585359. No behavior changes; tests unaffected (184 still green). - useHeadlessBuy.ts: document `getActiveSessionId()` semantics in the Fix #2 Design B routing block — calling `getQuotes` while an unrelated session is active will tear that session down. Safe today because of the single-live-session invariant enforced by `startHeadlessBuy`'s auto-cancel, but worth flagging for any future multi-session API. - types.ts: tighten the `onOrderCreated` JSDoc — clarify that the headless *session* terminates on order creation, not the underlying *order* (which may still be non-terminal). The original phrasing risked the exact misunderstanding Fix #3.2 is meant to prevent. - orderTerminalState.ts: reword the prerequisite-error message from "not in redux" to "not yet in redux" — acknowledges the addOrder-flush race the JSDoc already documents. - useTransakRouting.test.ts: tighten the bank-transfer fire-site assertion from `expect.objectContaining` to exact `refreshedOrder` identity, to match the native-card assertion. Catches any future drift that wraps or derives the order before passing it to `onOrderCreated`.
metamaskbotv2
Bot
removed
the
INVALID-PR-TEMPLATE
Two follow-ups to the Phase 9 fix bundle that close the remaining "Fix #2 follow-up" gap from the deferral list. Mirrors UB2's `useProviderLimits` behaviour for headless consumers (notably MMPay's `TransactionPayController`). - Suggestion #1: re-export `getProviderBuyLimit` (and its `ProviderBuyLimit` type) from `app/components/UI/Ramp/headless/index.ts`. Lets consumers do their own static bounds checks without importing from an internal utility path. Useful for e.g. disabling a "Get quote" button as the user types. - Suggestion #2: inside `useHeadlessBuy`'s `getQuotes`, run a no-network pre-flight check before the existing `getQuotesRaw` call. When `params.providerIds`, `params.paymentMethodIds`, and a fiat currency are all resolvable, and every `(provider × paymentMethodId)` candidate has known bounds that reject `params.amount`, short-circuit immediately with `LIMIT_EXCEEDED` instead of paying the network round-trip. Routes the rejection through `failSession` (Design B) when a session is active, mirroring the post-network rejection path so consumer UX is symmetric. Adds `currency?: string` to `HeadlessGetQuotesParams` so consumers can override the userRegion default when needed. Decision rule: short-circuit only when *every* candidate has known out-of-bounds. A single "passes" or "unknown bounds" candidate keeps the network call alive — preserves today's behaviour of letting the network decide when we can't be certain. Matches UB2's posture. Skip conditions (let the network decide): - `amount <= 0` or non-finite (UB2 parity — `useProviderLimits` returns `null` for non-positive amounts so the UI shows no error mid-typing). - `providerIds` missing/empty, `paymentMethodIds` missing/empty, fiat currency unresolvable, or the providers catalog hasn't loaded yet. - Any candidate's provider isn't in the catalog (unknown — let the network return the provider error; Fix #2's post-network path catches it). `details.source` discriminator added to both LIMIT_EXCEEDED paths (`static-bounds` for pre-flight, `network-reject` for post-network). Lets consumers branch on which layer caught the rejection without sniffing for the presence of `providerErrors` vs `rejections`. Test coverage: 53 tests in `describe('getQuotes', ...)` — happy paths, both rejection paths, Design B routing, boundary equality (amount === minAmount / === maxAmount → in-bounds), mix of known-reject + known-accept candidates, NaN, amount === 0, empty arrays, missing currency, missing provider catalog, `provider.limits === undefined`, an explicit `getProviderBuyLimit` index re-export check. 200 jest tests across `app/components/UI/Ramp/{headless,hooks,Views/Checkout}` all green. `yarn tsc --noEmit`, `yarn eslint`, `yarn prettier --check` all clean.
…gbot) The `@deprecated` JSDoc on `getOrderById` claimed it "forwards to the same implementation" as `getOrder`. That's false: the two methods use different selectors with different scoping. - `getOrderById` (via `useRampsOrders`) reads through `selectRampsOrdersForSelectedAccountGroup` — account-group-scoped, only returns orders whose `walletAddress` belongs to the current account group's wallets. - `getOrder` (via `orderTerminalState.ts`) reads through the unscoped `selectRampsOrders` selector — returns any matching order regardless of which account group owns it. A consumer following the deprecation guidance and switching from `getOrderById` to `getOrder` would silently lose account-group filtering (could surface an order from a different account they're not currently viewing). Behaviour itself is deliberate: Phase 9's `getOrder` is designed for non- React contexts (e.g. MetaMask Pay's `TransactionPayController`) that don't have a "selected account group" concept and would lose visibility into their own tracked orders if it scoped. So this commit is documentation- only — it makes the two contracts explicit so consumers know which one they need. Updated: - `types.ts` `HeadlessBuyResult.getOrderById` and `.getOrder` JSDocs. - `orderTerminalState.ts` imperative `getOrder` JSDoc. Filed by Cursor Bugbot (commit 4257d6a) on PR #30103.
Both filed against commit b2eb3fa after the Suggestion #1 + #2 push. 1) HeadlessPlayground: pass `walletAddress` to `awaitOrderTerminalState`. The "Await terminal state" button was calling `awaitOrderTerminalState` with only `{ timeoutMs }`. Combined with Fix #3.3's pre-flight check, tapping it during the brief window between `onOrderCreated` firing and `addOrder` flushing the order to redux would reject with `AwaitOrderTerminalStatePrerequisitesError`. The playground is supposed to be the reference implementation for the JSDoc-recommended pattern, so it now demonstrates that pattern: capture `order` from the widened `onOrderCreated(orderId, order)` signature (Fix #3.1), persist it in state, and pass `order.walletAddress` to `awaitOrderTerminalState`. Eliminates the race entirely. 2) `getQuotes` classification: aggregate-some across all provider errors leaked verdicts across providers. If provider A returned a buy-limit code and provider B returned a rate-limit code, the previous code computed `sdkCodeSaysLimit = true` AND `sdkCodeSaysRateRequest = true`, then evaluated `sdkCodeSaysLimit && !sdkCodeSaysRateRequest` → `false` → fell through to `QUOTE_FAILED` even though A clearly hit a buy bound. Now classifies per provider: a provider counts as a buy-limit signal only when ITS code says limit AND NOT rate/request — verdicts no longer leak across providers. Added a regression test that mixes `AMOUNT_LIMIT_EXCEEDED` (transak) with `RATE_LIMIT_EXCEEDED` (moonpay) and asserts the result is still `LIMIT_EXCEEDED`. Tests: 126 jest tests across the two touched suites all green. `yarn tsc --noEmit`, `yarn eslint`, `yarn prettier --check` clean.
…cov) Three pieces of bot feedback addressed in one commit: 1) Cursor Bugbot (Medium): classification leaked across providers on the message-regex fallback path. The previous fix only handled the SDK `code`-based path per-provider; the regex still ran on a `combinedMessage` joined across all provider errors. If provider A returned "Below minimum buy amount" (no SDK code) and provider B returned "Rate limit exceeded" (no SDK code), the combined message contained both "minimum" AND "rate" — flipping the result to `QUOTE_FAILED` even though A clearly hit a buy bound. Refactored to a single per-provider `some()` loop that classifies each provider's own `code` OR `message` independently. Added a regression test. 2) SonarCloud (Major, typescript:S6759): `OrderTrackingPanelProps` had mutable props. React-component prop interfaces should be read-only — marked all seven fields `readonly`. 3) Codecov: the existing "unrecognized providerId" test used `providers: []` which short-circuits at the catalog-empty guard before reaching the unknown-provider branch inside the candidate loop. Added a sibling test where the catalog has SOME entries (just not the requested one) so the `if (!provider)` branch and its `bounds: undefined` path actually execute. Closes the `useHeadlessBuy.ts:134-137` coverage gap. Tests: 193 jest tests across the touched suites all green. `yarn tsc --noEmit`, `yarn eslint`, `yarn prettier --check` clean.
Phase 9 added orderTerminalState.ts, which re-implements two methods that already exist in useRampsOrders (getOrderById and refreshOrder) plus a new awaitOrderTerminalState polling primitive. The deviation from Pedro's original facade pattern (Phase 2, #29144) was motivated by the assumption that a non-React, cross-account-group consumer needed an imperative API. No such consumer exists in Headless Buy's consumer set — the one known consumer (useFiatConfirm in #28152) uses only startHeadlessBuy, and order status display (useFiatOrderStatus in the same PR) polls Engine.context.RampsController.getOrder directly via setInterval. This commit reverts the deviation. Removed: - orderTerminalState.ts + tests (re-implementations of useRampsOrders primitives, motivated by a phantom non-React consumer) - useHeadlessBuy passthroughs (getOrder, refreshOrder, awaitOrderTerminalState) - Playground OrderTrackingPanel + testIDs + styles - i18n order_tracking_* keys (en.json only — never propagated) - Fix #3.3 pre-flight error (only existed to harden the removed function) - Phase 9 deprecation JSDoc on the pre-existing getOrderById Kept: - Fix #2 (provider-rejection routing through onError) - Fix #3.1 (onOrderCreated(orderId, order) widening) - Fix #3.2 (asymmetric-callback JSDoc + per-path freshness table, pared) - Suggestion #1 (getProviderBuyLimit re-export) - Suggestion #2 (pre-quote static bounds short-circuit) - Analytics + deeplink utilities - sessionRegistry Fix #2 routing - Pedro's pre-existing getOrderById on useHeadlessBuy (with Phase 9's deprecation JSDoc reverted to its pre-Phase-9 form) Note for future maintainers: - The Hermes-safe Object.setPrototypeOf pattern for typed errors is no longer exercised in the headless module's tests. Reintroduce the pattern if a future typed error is added. - Fix #3.1 propagation coverage at the playground layer is removed, but the propagation itself remains covered by Checkout / useTransakRouting fire-site tests.
saustrie-consensys
changed the title
saustrie-consensys
changed the title
saustrie-consensys
removed
the
pr-not-ready-for-e2e
Contributor
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 5a02bbd. Configure here.
Contributor
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
These changes directly affect the on-ramp/buy flow (unified buy, Transak routing, checkout WebView), which is covered by Performance Test Selection: |
|
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.
Description
Fixes from Goktug's May 12 headless-buy testing thread, plus a small utility MetaMask Pay needs for mid-typing validation.
Independent of Phase 9.5 (#30104) in runtime — #30104 is currently git-stacked on this branch, so GitHub will auto-retarget its base to
mainwhen this PR merges. Either can land first.Fixes from Goktug's May 12 testing
Three issues he filed; the fourth (empty white screen) is scoped to PR #30104.
getQuotesno longer silently eats provider rejections. A 5 EUR amount under the provider's minimum used to return an empty success list and no error, so the consumer was stuck waiting. NowgetQuotesthrows a typedHeadlessBuyErrorAND firesonError+onCloseon the active session. The error code isLIMIT_EXCEEDEDfor limit messages andQUOTE_FAILEDfor everything else (rate-limit messages are NOT mis-classified as limit-exceeded).onOrderCreatednow hands the consumer the full order. Was(orderId) => void, now(orderId, order: RampsOrder) => void. Consumers can readorder.walletAddress/order.provider.idstraight from the callback without a separate lookup.onErrordoes NOT fire when an order is created and then later fails (e.g. 3-D Secure rejection on a card). Consumers need to branch on the order's.status, or poll for settlement themselves —useFiatOrderStatusin feat: Add fiat payment confirmation flow with headless ramp integration placeholder #28152 is the canonical pattern (pollsRampsController.getOrderdirectly).Pre-quote static bounds (Suggestions #1 + #2 — MetaMask Pay)
Originally deferred; MMPay confirmed they want both pieces.
getProviderBuyLimitfrom the publicheadlessbarrel. Lets a consumer check whether a typed-in amount is in-bounds for(provider, currency, paymentMethod)without a network call — handy for disabling a "Get quote" button while the user types.getQuotesruns that same check before paying the network round-trip. If every requested provider × payment-method has known bounds that reject the amount, it rejects synchronously withLIMIT_EXCEEDED. A single "passes" or "unknown bounds" candidate keeps the network call alive — same conservative posture UB2 takes.Out of scope / deferred:
getUserLimitsswallow atuseTransakRouting.ts:241— not in Goktug's filed complaints; tracked separately.navigateToOrderProcessingCallbackis unrelated (different session machinery) — intentionally skipped.useFiatConfirm/useFiatOrderStatusconsuming the newonOrderCreatedarg +getProviderBuyLimit) — lives in their own PRs.Changelog
CHANGELOG entry: null
(internal API addition — no end-user-facing change in this PR; MMPay's user-visible loading UI lands in their downstream PRs.)
Related issues
Fixes: https://consensyssoftware.atlassian.net/jira/software/c/projects/TRAM/boards/1568?assignee=712020%3Afd12f7ea-d9e1-4a0a-8a26-36804c9e11c9&selectedIssue=TRAM-3530
Replaces the relevant Phase 9 scope of the combined PR #29930 (close after this + #30104 are open).
Parallel Phase 9.5 PR: #30104 (HeadlessHost goes invisible). No runtime merge-order dependency; #30104 is git-stacked on this branch.
Slack — Goktug's May 12 testing thread: https://consensys.slack.com/archives/C0AK3NXRM7W/p1778577403631269
Manual testing steps
Screenshots/Recordings
N/A — internal API addition. No user-facing UI change in this PR.
Before
N/A
After
N/A
Pre-merge author checklist
app/components/UI/Ramp/{headless,hooks,Views/Checkout,Views/HeadlessPlayground}; covers Fix Change inpage/inpage.js -> entry/entry.js #2 response-inspection + classification, Fix Implement loading spinner during page loads #3.1 fire-site 2nd-arg passthrough at Checkout anduseTransakRouting, and Suggestion Change inpage/inpage.js -> entry/entry.js #2 static bounds short-circuit with mixed-candidate edge cases)team-money-movement)Performance checks (if applicable)
(N/A for this phase — internal API addition. The next consumer-facing change lands in MMPay's downstream PR.)
Pre-merge reviewer checklist
Note
Medium Risk
Expands the headless public callback API and changes
useHeadlessBuy.getQuotesbehavior to throw and auto-fail active sessions, which could break existing consumers and alter control flow around quote fetching and session lifecycle.Overview
Headless buy now surfaces quote failures instead of silently returning empty results:
useHeadlessBuy.getQuotesthrows typed errors (LIMIT_EXCEEDEDvsQUOTE_FAILED), and when a headless session is active it routes these throughfailSessionto triggeronError+onCloseand end the session.Adds a pre-network static bounds check using provider catalog limits; if all requested provider×payment-method candidates have known bounds that reject the amount,
getQuotesshort-circuits withLIMIT_EXCEEDED.getProviderBuyLimitis also re-exported from the publicheadlessbarrel for consumer-side validation.Widens the headless session callback signature to
onOrderCreated(orderId, order)and plumbs the order snapshot through the headless success paths (Checkoutcallback flow anduseTransakRouting), updating the playground and tests accordingly, plus clarifying JSDoc about post-creation order status behavior.Reviewed by Cursor Bugbot for commit e0a95a4. Bugbot is set up for automated code reviews on this repo. Configure here.