feat: retry failed quote loads and surface quote errors in TransactionPayController - #9837
feat: retry failed quote loads and surface quote errors in TransactionPayController#9837dan437 wants to merge 10 commits into
Conversation
A single failed quote fetch permanently stranded a transaction without quotes: errors were swallowed silently and the refresh loop skipped transactions with no quotes. Now failures are persisted to quoteError and the refresh loop retries transactions that need quotes but have none.
…retry-failed-quote-loads
| subscribeTransactionChanges, | ||
| } from './utils/transaction.js'; | ||
|
|
||
| const log = createModuleLogger(projectLogger, 'controller'); |
There was a problem hiding this comment.
The projectLogger is the log function itself for the main controller file.
| transaction.quotes?.some( | ||
| (quote) => quote.strategy !== TransactionPayStrategy.None, | ||
| ), | ||
| ) || isQuoteRetryPending(transaction), |
There was a problem hiding this comment.
As discussed, we need to confirm this is some Barbara wants.
My original assumption was the error cases (provider or simulation errors) wouldn't fix themselves so redundant to have the extra traffic and UX skeletons etc.
| chainId: quotes[0].request.sourceChainId, | ||
| messenger, | ||
| })) ?? DEFAULT_REFRESH_INTERVAL; | ||
| const refreshInterval = firstQuote |
There was a problem hiding this comment.
Minor, could we assign default then override if quotes and strategy interval, rather than ternary and fallbacks?
|
|
||
| const firstQuote = quotes?.[0]; | ||
|
|
||
| if (!firstQuote && !isQuoteRetryPending(transactionData)) { |
There was a problem hiding this comment.
Could we replace isQuoteRetryPending with just checking if quoteError is set as we always set that?
| transactionId, | ||
| updateTransactionData, | ||
| (data) => { | ||
| data.quotesLastUpdated = Date.now(); |
There was a problem hiding this comment.
Why do we need to update this if we haven't derived new quotes?
What does this enable in the UI?
|
Closing this for now. Mobile #34471 fixes the user-facing issue, while this retry logic also retries legitimate empty responses. |
2 participants
Explanation
When a quote load in
TransactionPayControllerfailed unexpectedly, the error was swallowed (.catch(noop)), nothing was written to state, and the refresh loop skipped transactions with no quotes — so they were never retried. One failed fetch left the transaction permanently without quotes: clients showed no fees, and submission was blocked by the publish guard ("MetaMask Pay: Cannot submit without quote").This change makes failed quote loads recoverable:
refreshQuotesnow also retries transactions that should have quotes but have none, on the default refresh interval. One transaction's failure no longer stops the others from refreshing.QuoteRefresherkeeps the refresh loop alive while such a transaction exists.quoteError(reasonno-quotes) instead of being swallowed — but only when quotes are needed and none are usable. Existing executable quotes stay usable until a refresh replaces them, and direct routes stay silent because they need no quotes. Stale no-op quotes from a previous direct route are dropped so the retry loop picks the transaction up.quotesLastUpdated, so retries wait a full interval instead of running every tick, and late writes cannot recreate pay state that cleanup already removed.References
Checklist
Note
Medium Risk
Touches core quote fetch, refresh, and error state for pay submission; behavior is well-tested but affects when users see errors and when submit is allowed.
Overview
Fixes MetaMask Pay transactions that could stay permanently without quotes after a single failed or empty quote fetch, which blocked fees display and submit.
Quote retry and refresh loop:
refreshQuotesnow retries transactions that should have quotes but have none (failed load, empty result, fiat payment selected, etc.), respecting the normal refresh interval viaquotesLastUpdated.QuoteRefresherkeeps polling whileisQuoteRetryPendingis true, not only when executable quotes already exist. Per-transaction failures are isolated so one bad transaction does not stop others from refreshing.Failure persistence: Unexpected quote pipeline errors call
persistQuoteLoadFailure, which stampsquotesLastUpdated, setsquoteError(no-quotes) only when quotes are needed and none are usable, drops stale no-op quotes so retries can run, and skips writes if pay state was removed. Refresh paths that skip or throw still stamp attempt time so retries do not hammer every tick.Controller:
updateQuotesrejections are logged instead of swallowed withnoop, without changing the fire-and-forget call pattern.Reviewed by Cursor Bugbot for commit b94dcdd. Bugbot is set up for automated code reviews on this repo. Configure here.