From a36d9c4d049062778fa7165cbe039a3317ca2353 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:29:33 +0200 Subject: [PATCH 1/8] feat: retry failed or empty quote loads in TransactionPayController 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. --- .../src/TransactionPayController.test.ts | 15 ++ .../src/TransactionPayController.ts | 11 +- .../src/helpers/QuoteRefresher.test.ts | 21 +- .../src/helpers/QuoteRefresher.ts | 19 +- .../src/utils/quotes.test.ts | 250 +++++++++++++++++- .../src/utils/quotes.ts | 89 +++++-- 6 files changed, 376 insertions(+), 29 deletions(-) diff --git a/packages/transaction-pay-controller/src/TransactionPayController.test.ts b/packages/transaction-pay-controller/src/TransactionPayController.test.ts index 467f6406ab2..2cc7ebf665b 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.test.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.test.ts @@ -3,6 +3,7 @@ import type { TransactionMeta } from '@metamask/transaction-controller'; import type { Hex } from '@metamask/utils'; +import { flushPromises } from '../../../tests/helpers.js'; import { updateFiatPayment } from './actions/update-fiat-payment.js'; import { updatePaymentToken } from './actions/update-payment-token.js'; import { PaymentOverride, TransactionPayStrategy } from './constants.js'; @@ -262,6 +263,20 @@ describe('TransactionPayController', () => { expect(updateQuotesMock).toHaveBeenCalledTimes(1); }); + it('does not throw when quote update fails', async () => { + const controller = createController(); + + updateQuotesMock.mockRejectedValueOnce(new Error('Quote update failed')); + + controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => { + // no-op, just initializes + }); + + await flushPromises(); + + expect(updateQuotesMock).toHaveBeenCalledTimes(1); + }); + it('updates refundTo in state', () => { const controller = createController(); const refundTo = '0xdeadbeef00000000000000000000000000000001' as Hex; diff --git a/packages/transaction-pay-controller/src/TransactionPayController.ts b/packages/transaction-pay-controller/src/TransactionPayController.ts index 0f35951e683..bc6eb0d12a6 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.ts @@ -1,8 +1,8 @@ import type { StateMetadata } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import type { TransactionMeta } from '@metamask/transaction-controller'; +import { createModuleLogger } from '@metamask/utils'; import type { Draft } from 'immer'; -import { noop } from 'lodash'; import { updateFiatPayment } from './actions/update-fiat-payment.js'; import { updatePaymentToken } from './actions/update-payment-token.js'; @@ -12,6 +12,7 @@ import { TransactionPayStrategy, } from './constants.js'; import { QuoteRefresher } from './helpers/QuoteRefresher.js'; +import { projectLogger } from './logger.js'; import type { GetAmountDataCallback, GetDelegationTransactionCallback, @@ -36,6 +37,8 @@ import { subscribeTransactionChanges, } from './utils/transaction.js'; +const log = createModuleLogger(projectLogger, 'controller'); + const MESSENGER_EXPOSED_METHODS = [ 'getAmountData', 'getDelegationTransaction', @@ -396,7 +399,11 @@ export class TransactionPayController extends BaseController< transactionData: this.state.transactionData[transactionId], transactionId, updateTransactionData: this.#updateTransactionData.bind(this), - }).catch(noop); + }).catch((error) => { + // The failure is persisted as quoteError by updateQuotes and retried + // by the refresh loop. + log('Failed to update quotes', { transactionId, error }); + }); } } diff --git a/packages/transaction-pay-controller/src/helpers/QuoteRefresher.test.ts b/packages/transaction-pay-controller/src/helpers/QuoteRefresher.test.ts index cd95a2995f2..da5b1f030b6 100644 --- a/packages/transaction-pay-controller/src/helpers/QuoteRefresher.test.ts +++ b/packages/transaction-pay-controller/src/helpers/QuoteRefresher.test.ts @@ -9,7 +9,7 @@ import type { TransactionData, TransactionPayControllerMessenger, } from '../types.js'; -import { refreshQuotes } from '../utils/quotes.js'; +import { isQuoteRetryPending, refreshQuotes } from '../utils/quotes.js'; import { QuoteRefresher } from './QuoteRefresher.js'; jest.mock('../utils/quotes'); @@ -18,6 +18,7 @@ jest.useFakeTimers(); describe('QuoteRefresher', () => { const refreshQuotesMock = jest.mocked(refreshQuotes); + const isQuoteRetryPendingMock = jest.mocked(isQuoteRetryPending); let messenger: TransactionPayControllerMessenger; let publish: ReturnType['publish']; @@ -48,6 +49,7 @@ describe('QuoteRefresher', () => { ({ messenger, publish } = getMessengerMock()); refreshQuotesMock.mockResolvedValue(undefined); + isQuoteRetryPendingMock.mockReturnValue(false); }); it('polls if quotes detected in state', async () => { @@ -80,6 +82,23 @@ describe('QuoteRefresher', () => { expect(refreshQuotesMock).not.toHaveBeenCalled(); }); + it('polls if a transaction needs quotes but has none', async () => { + new QuoteRefresher({ + getStrategies: jest.fn().mockReturnValue([TransactionPayStrategy.Relay]), + messenger, + updateTransactionData: jest.fn(), + }); + + isQuoteRetryPendingMock.mockReturnValue(true); + + publishStateChange({ hasQuotes: false }); + + jest.runAllTimers(); + await flushPromises(); + + expect(refreshQuotesMock).toHaveBeenCalledTimes(1); + }); + it('does not poll if only no-op quotes in state', async () => { new QuoteRefresher({ getStrategies: jest.fn().mockReturnValue([TransactionPayStrategy.Relay]), diff --git a/packages/transaction-pay-controller/src/helpers/QuoteRefresher.ts b/packages/transaction-pay-controller/src/helpers/QuoteRefresher.ts index f90e2e58c3e..e90a370299a 100644 --- a/packages/transaction-pay-controller/src/helpers/QuoteRefresher.ts +++ b/packages/transaction-pay-controller/src/helpers/QuoteRefresher.ts @@ -9,7 +9,7 @@ import type { } from '../index.js'; import { projectLogger } from '../logger.js'; import type { UpdateTransactionDataCallback } from '../types.js'; -import { refreshQuotes } from '../utils/quotes.js'; +import { isQuoteRetryPending, refreshQuotes } from '../utils/quotes.js'; const CHECK_INTERVAL = 1000; // 1 Second @@ -107,15 +107,20 @@ export class QuoteRefresher { #onStateChange(state: TransactionPayControllerState): void { // No-op quotes never refresh, so they don't need the refresh loop. - const hasQuotes = Object.values(state.transactionData).some((transaction) => - transaction.quotes?.some( - (quote) => quote.strategy !== TransactionPayStrategy.None, - ), + // Transactions that need quotes but have none had a failed or empty + // quote load and need the loop to retry them. + const needsRefreshLoop = Object.values(state.transactionData).some( + (transaction) => + Boolean( + transaction.quotes?.some( + (quote) => quote.strategy !== TransactionPayStrategy.None, + ), + ) || isQuoteRetryPending(transaction), ); - if (hasQuotes && !this.#isRunning) { + if (needsRefreshLoop && !this.#isRunning) { this.#start(); - } else if (!hasQuotes && this.#isRunning) { + } else if (!needsRefreshLoop && this.#isRunning) { this.#stop(); } } diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index c0135333d94..259610066d1 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -15,7 +15,7 @@ import type { TransactionPayRequiredToken, } from '../types.js'; import type { UpdateQuotesRequest } from './quotes.js'; -import { refreshQuotes, updateQuotes } from './quotes.js'; +import { isQuoteRetryPending, refreshQuotes, updateQuotes } from './quotes.js'; import { checkStrategyQuoteSupport, checkStrategySupport, @@ -1316,6 +1316,31 @@ describe('Quotes Utils', () => { await expect(run()).rejects.toThrow(pipelineError); }); + it('persists quote error and refresh timestamp when the quote pipeline throws', async () => { + getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); + calculateTotalsMock.mockImplementationOnce(() => { + throw new Error('calculateTotals failed'); + }); + + await expect(run()).rejects.toThrow('calculateTotals failed'); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock).toMatchObject({ + isLoading: false, + quoteError: { + message: 'calculateTotals failed', + reason: 'no-quotes', + }, + }); + expect(transactionDataMock.quotesLastUpdated).toStrictEqual( + expect.any(Number), + ); + }); + it('catches abort errors thrown by strategy and returns false', async () => { const strategyError = new Error('The operation was aborted'); const gate = deferred[]>(); @@ -1526,6 +1551,229 @@ describe('Quotes Utils', () => { expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); }); + + it('retries a transaction that needs quotes but has none', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: 1, + } as TransactionData, + }, + }); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock).toMatchObject({ + quotes: [ + expect.objectContaining({ + strategy: TransactionPayStrategy.Across, + }), + ], + }); + }); + + it('retries a transaction with a selected fiat payment method and no quotes', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + isLoading: false, + fiatPayment: { selectedPaymentMethodId: 'method-1' }, + quotes: [], + quotesLastUpdated: 1, + tokens: [], + } as unknown as TransactionData, + }, + }); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock).toMatchObject({ + quotes: [ + expect.objectContaining({ + strategy: TransactionPayStrategy.Across, + }), + ], + }); + }); + + it('does not retry a transaction with no quotes when none are needed', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + isLoading: false, + quotes: [], + } as unknown as TransactionData, + }, + }); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); + }); + + it('does not retry a transaction with no quotes before the refresh interval', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: Date.now(), + } as TransactionData, + }, + }); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); + }); + + it('does not log a refresh when the update is superseded', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: 1, + } as TransactionData, + }, + }); + + // updateQuotes returns false when the transaction is not unapproved. + getTransactionMock.mockReturnValueOnce({ + ...TRANSACTION_META_MOCK, + status: TransactionStatus.submitted, + } as TransactionMeta); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); + }); + + it('continues refreshing other transactions when one update fails', async () => { + const secondTransactionId = '234-567'; + + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: 1, + } as TransactionData, + [secondTransactionId]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: 1, + } as TransactionData, + }, + }); + + // First transaction fails inside updateQuotes. + getTransactionMock.mockReturnValueOnce(undefined); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock).toMatchObject({ + quotes: [ + expect.objectContaining({ + strategy: TransactionPayStrategy.Across, + }), + ], + }); + }); + }); + + describe('isQuoteRetryPending', () => { + it('returns false when quotes exist', () => { + expect( + isQuoteRetryPending({ + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [QUOTE_MOCK], + } as TransactionData), + ).toBe(false); + }); + + it('returns true when payment token has pending conversions and no quotes', () => { + expect( + isQuoteRetryPending({ + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + } as TransactionData), + ).toBe(true); + }); + + it('returns true when fiat payment method is selected and no quotes', () => { + expect( + isQuoteRetryPending({ + fiatPayment: { selectedPaymentMethodId: 'method-1' }, + quotes: [], + } as unknown as TransactionData), + ).toBe(true); + }); + + it('returns false when payment token has no pending conversions', () => { + expect( + isQuoteRetryPending({ + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: undefined, + sourceAmounts: [], + } as TransactionData), + ).toBe(false); + }); + + it('returns false when payment token set but source amounts missing', () => { + expect( + isQuoteRetryPending({ + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: undefined, + sourceAmounts: undefined, + } as TransactionData), + ).toBe(false); + }); + + it('returns false when no payment token or fiat payment method', () => { + expect(isQuoteRetryPending({} as TransactionData)).toBe(false); + }); }); describe('post-quote (withdrawal) flow', () => { diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 48cf9ff93f8..2c2ef247ee8 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -200,6 +200,18 @@ export async function updateQuotes( log('Quote request aborted', { transactionId, reason: signal.reason }); return false; } + + // Persist the failure so clients can surface it, and stamp the update + // time so the refresh loop retries on the normal interval instead of + // every tick. + updateTransactionData(transactionId, (data) => { + data.quoteError = { + message: (error as Error).message, + reason: 'no-quotes', + }; + data.quotesLastUpdated = Date.now(); + }); + throw error; } finally { if (!signal.aborted) { @@ -291,9 +303,37 @@ function syncTransaction({ ); } +/** + * Determine whether a transaction is waiting on a quote retry: it needs + * quotes — a payment token with pending conversions, or a selected fiat + * payment method — but has none, meaning the last quote load failed or + * returned nothing. + * + * @param transactionData - Transaction data to check. + * @returns True when the transaction needs quotes but has none. + */ +export function isQuoteRetryPending( + transactionData: TransactionData, +): boolean { + const { fiatPayment, paymentToken, quotes, sourceAmounts } = transactionData; + + if (quotes?.length) { + return false; + } + + return ( + (Boolean(paymentToken) && (sourceAmounts?.length ?? 0) > 0) || + Boolean(fiatPayment?.selectedPaymentMethodId) + ); +} + /** * Refresh quotes for all transactions if expired. * + * Also retries transactions that need quotes but have none — a failed or + * empty quote load — so a transient failure does not permanently strand a + * transaction without quotes. + * * @param messenger - Messenger instance. * @param updateTransactionData - Callback to update transaction data. * @param getStrategies - Callback to get ordered strategy names for a transaction. @@ -310,26 +350,33 @@ export async function refreshQuotes( const transactionData = state.transactionData[transactionId]; const { isLoading, quotes, quotesLastUpdated } = transactionData; - if (isLoading || !quotes?.length) { + if (isLoading) { + continue; + } + + const firstQuote = quotes?.[0]; + + if (!firstQuote && !isQuoteRetryPending(transactionData)) { continue; } // No-op quotes mark direct routes and have nothing to refresh. They are // regenerated whenever the transaction data changes. if ( - quotes.every((quote) => quote.strategy === TransactionPayStrategy.None) + firstQuote && + quotes?.every((quote) => quote.strategy === TransactionPayStrategy.None) ) { continue; } - const strategyName = quotes[0].strategy; - const strategy = getStrategyByName(strategyName); + const strategyName = firstQuote?.strategy; - const refreshInterval = - (await strategy.getRefreshInterval?.({ - chainId: quotes[0].request.sourceChainId, - messenger, - })) ?? DEFAULT_REFRESH_INTERVAL; + const refreshInterval = firstQuote + ? ((await getStrategyByName(firstQuote.strategy).getRefreshInterval?.({ + chainId: firstQuote.request.sourceChainId, + messenger, + })) ?? DEFAULT_REFRESH_INTERVAL) + : DEFAULT_REFRESH_INTERVAL; const isExpired = Date.now() - (quotesLastUpdated ?? 0) > refreshInterval; @@ -337,16 +384,22 @@ export async function refreshQuotes( continue; } - const isUpdated = await updateQuotes({ - getStrategies, - messenger, - transactionData, - transactionId, - updateTransactionData, - }); + try { + const isUpdated = await updateQuotes({ + getStrategies, + messenger, + transactionData, + transactionId, + updateTransactionData, + }); - if (isUpdated) { - log('Refreshed quotes', { transactionId, strategy: strategyName }); + if (isUpdated) { + log('Refreshed quotes', { transactionId, strategy: strategyName }); + } + } catch (error) { + // The failure is persisted by updateQuotes; keep refreshing the + // remaining transactions. + log('Failed to refresh quotes', { transactionId, error }); } } } From 61f1e7686388751b48db6ce16217a42f95d39759 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:36:18 +0200 Subject: [PATCH 2/8] feat: stamp quote retry attempts so failed retries wait a full interval --- .../src/utils/quotes.test.ts | 47 ++++++++++++++++++- .../src/utils/quotes.ts | 18 ++++++- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index 259610066d1..6e548e8e88b 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -1655,7 +1655,7 @@ describe('Quotes Utils', () => { expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); }); - it('does not log a refresh when the update is superseded', async () => { + it('stamps the attempt time when the update reports no change', async () => { getControllerStateMock.mockReturnValue({ transactionData: { [TRANSACTION_ID_MOCK]: { @@ -1678,7 +1678,50 @@ describe('Quotes Utils', () => { getStrategiesMock, ); - expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); + // Only the attempt stamp is written so the next retry waits a full + // refresh interval instead of running on every tick. + expect(updateTransactionDataMock).toHaveBeenCalledTimes(1); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock).toStrictEqual({ + quotesLastUpdated: expect.any(Number), + }); + }); + + it('stamps the attempt time when the update throws before starting', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: 1, + } as TransactionData, + }, + }); + + // updateQuotes throws when the transaction cannot be found. + getTransactionMock.mockReturnValueOnce(undefined); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + expect(updateTransactionDataMock).toHaveBeenCalledTimes(1); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock).toStrictEqual({ + quotesLastUpdated: expect.any(Number), + }); }); it('continues refreshing other transactions when one update fails', async () => { diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 2c2ef247ee8..2cc28b4abfe 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -395,15 +395,29 @@ export async function refreshQuotes( if (isUpdated) { log('Refreshed quotes', { transactionId, strategy: strategyName }); + } else { + // Not updated (e.g. transaction no longer unapproved): stamp the + // attempt so the next one waits a full interval, not every tick. + stampQuotesLastUpdated(transactionId, updateTransactionData); } } catch (error) { - // The failure is persisted by updateQuotes; keep refreshing the - // remaining transactions. + // Also stamp throws that skip the updateQuotes catch, such as + // "Transaction not found". Keep refreshing the remaining transactions. + stampQuotesLastUpdated(transactionId, updateTransactionData); log('Failed to refresh quotes', { transactionId, error }); } } } +function stampQuotesLastUpdated( + transactionId: string, + updateTransactionData: UpdateTransactionDataCallback, +): void { + updateTransactionData(transactionId, (data) => { + data.quotesLastUpdated = Date.now(); + }); +} + function abortPreviousAndCreateController( transactionId: string, ): AbortController { From d1280af4a31281ac1950ad4eb8bc97fe6f7668fa Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:45:28 +0200 Subject: [PATCH 3/8] feat: skip late quote writes when pay state was removed --- .../src/utils/quotes.test.ts | 48 ++++++++++++++ .../src/utils/quotes.ts | 64 +++++++++++++++---- 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index 6e548e8e88b..eaa2821de2e 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -187,6 +187,11 @@ describe('Quotes Utils', () => { }, ); + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: cloneDeep(TRANSACTION_DATA_MOCK), + }, + }); getTransactionMock.mockReturnValue(TRANSACTION_META_MOCK); getQuotesMock.mockResolvedValue([QUOTE_MOCK]); getBatchTransactionsMock.mockResolvedValue([BATCH_TRANSACTION_MOCK]); @@ -1341,6 +1346,24 @@ describe('Quotes Utils', () => { ); }); + it('does not persist quote error when pay state was removed', async () => { + getControllerStateMock.mockReturnValue({ transactionData: {} }); + getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); + calculateTotalsMock.mockImplementationOnce(() => { + throw new Error('calculateTotals failed'); + }); + + await expect(run()).rejects.toThrow('calculateTotals failed'); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock.quoteError).toBeUndefined(); + expect(transactionDataMock.quotesLastUpdated).toBeUndefined(); + }); + it('catches abort errors thrown by strategy and returns false', async () => { const strategyError = new Error('The operation was aborted'); const gate = deferred[]>(); @@ -1724,6 +1747,31 @@ describe('Quotes Utils', () => { }); }); + it('does not stamp the attempt when pay state was removed', async () => { + getControllerStateMock + .mockReturnValueOnce({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [], + quotesLastUpdated: 1, + } as TransactionData, + }, + }) + .mockReturnValue({ transactionData: {} }); + + // updateQuotes throws when the transaction cannot be found. + getTransactionMock.mockReturnValueOnce(undefined); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + expect(updateTransactionDataMock).toHaveBeenCalledTimes(0); + }); + it('continues refreshing other transactions when one update fails', async () => { const secondTransactionId = '234-567'; diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 2cc28b4abfe..3642df01b1c 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -3,6 +3,7 @@ import type { BatchTransaction } from '@metamask/transaction-controller'; import type { TransactionMeta } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; +import type { Draft } from 'immer'; import { PaymentOverride, TransactionPayStrategy } from '../constants.js'; import { projectLogger } from '../logger.js'; @@ -204,13 +205,18 @@ export async function updateQuotes( // Persist the failure so clients can surface it, and stamp the update // time so the refresh loop retries on the normal interval instead of // every tick. - updateTransactionData(transactionId, (data) => { - data.quoteError = { - message: (error as Error).message, - reason: 'no-quotes', - }; - data.quotesLastUpdated = Date.now(); - }); + updateExistingTransactionData( + messenger, + transactionId, + updateTransactionData, + (data) => { + data.quoteError = { + message: (error as Error).message, + reason: 'no-quotes', + }; + data.quotesLastUpdated = Date.now(); + }, + ); throw error; } finally { @@ -398,24 +404,58 @@ export async function refreshQuotes( } else { // Not updated (e.g. transaction no longer unapproved): stamp the // attempt so the next one waits a full interval, not every tick. - stampQuotesLastUpdated(transactionId, updateTransactionData); + stampQuotesLastUpdated(messenger, transactionId, updateTransactionData); } } catch (error) { // Also stamp throws that skip the updateQuotes catch, such as // "Transaction not found". Keep refreshing the remaining transactions. - stampQuotesLastUpdated(transactionId, updateTransactionData); + stampQuotesLastUpdated(messenger, transactionId, updateTransactionData); log('Failed to refresh quotes', { transactionId, error }); } } } function stampQuotesLastUpdated( + messenger: TransactionPayControllerMessenger, transactionId: string, updateTransactionData: UpdateTransactionDataCallback, ): void { - updateTransactionData(transactionId, (data) => { - data.quotesLastUpdated = Date.now(); - }); + updateExistingTransactionData( + messenger, + transactionId, + updateTransactionData, + (data) => { + data.quotesLastUpdated = Date.now(); + }, + ); +} + +/** + * Update transaction data only when the entry still exists in state, so a + * late write cannot recreate an entry that cleanup already removed. + * + * @param messenger - Messenger instance. + * @param transactionId - ID of the transaction to update. + * @param updateTransactionData - Callback to update transaction data. + * @param fn - Function that receives a draft of the transaction data. + */ +function updateExistingTransactionData( + messenger: TransactionPayControllerMessenger, + transactionId: string, + updateTransactionData: UpdateTransactionDataCallback, + fn: (data: Draft) => void, +): void { + const exists = Boolean( + messenger.call('TransactionPayController:getState').transactionData[ + transactionId + ], + ); + + if (!exists) { + return; + } + + updateTransactionData(transactionId, fn); } function abortPreviousAndCreateController( From a2c4b296a6b0ccfb323373d9603329123942cdb1 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:53:15 +0200 Subject: [PATCH 4/8] feat: drop stale no-op quotes on failed quote loads so retries can run --- .../src/utils/quotes.test.ts | 38 +++++++++++++++++++ .../src/utils/quotes.ts | 12 ++++++ 2 files changed, 50 insertions(+) diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index eaa2821de2e..814b53e3988 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -1346,6 +1346,44 @@ describe('Quotes Utils', () => { ); }); + it('drops stale no-op quotes when the quote pipeline throws', async () => { + getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); + calculateTotalsMock.mockImplementationOnce(() => { + throw new Error('calculateTotals failed'); + }); + + await expect(run()).rejects.toThrow('calculateTotals failed'); + + // Seed the draft with a no-op quote from a previous direct route. + const transactionDataMock: Record = { + quotes: [{ strategy: TransactionPayStrategy.None }], + }; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock.quotes).toStrictEqual([]); + }); + + it('keeps executable quotes when the quote pipeline throws', async () => { + getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); + calculateTotalsMock.mockImplementationOnce(() => { + throw new Error('calculateTotals failed'); + }); + + await expect(run()).rejects.toThrow('calculateTotals failed'); + + // Seed the draft with an executable quote from a previous refresh. + const transactionDataMock: Record = { + quotes: [QUOTE_MOCK], + }; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + expect(transactionDataMock.quotes).toStrictEqual([QUOTE_MOCK]); + }); + it('does not persist quote error when pay state was removed', async () => { getControllerStateMock.mockReturnValue({ transactionData: {} }); getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 3642df01b1c..e7f5897667a 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -215,6 +215,18 @@ export async function updateQuotes( reason: 'no-quotes', }; data.quotesLastUpdated = Date.now(); + + // Stale no-op quotes from a previous direct route would block the + // retry loop, so drop them. Executable quotes stay usable until a + // refresh replaces them. + if ( + data.quotes?.length && + data.quotes.every( + (quote) => quote.strategy === TransactionPayStrategy.None, + ) + ) { + data.quotes = []; + } }, ); From 861555ea0b2b9f0a8f064eb03dfb0f27e40e0c87 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 12:01:05 +0200 Subject: [PATCH 5/8] feat: treat failed quote loads as retry-pending so direct routes recover --- .../src/utils/quotes.test.ts | 43 +++++++++++++++++++ .../src/utils/quotes.ts | 13 +++--- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index 814b53e3988..1fbd223cecc 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -1644,6 +1644,40 @@ describe('Quotes Utils', () => { }); }); + it('retries a direct-route transaction after a failed load', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quoteError: { message: 'Failed', reason: 'no-quotes' }, + quotes: [], + quotesLastUpdated: 1, + sourceAmounts: [], + } as TransactionData, + }, + }); + + await refreshQuotes( + messenger, + updateTransactionDataMock, + getStrategiesMock, + ); + + const transactionDataMock: Record = {}; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + // The retry regenerates the no-op quote for the direct route. + expect(transactionDataMock).toMatchObject({ + quotes: [ + expect.objectContaining({ + strategy: TransactionPayStrategy.None, + }), + ], + }); + }); + it('retries a transaction with a selected fiat payment method and no quotes', async () => { getControllerStateMock.mockReturnValue({ transactionData: { @@ -1871,6 +1905,15 @@ describe('Quotes Utils', () => { ).toBe(true); }); + it('returns true when the last quote load failed and there are no quotes', () => { + expect( + isQuoteRetryPending({ + quoteError: { message: 'Failed', reason: 'no-quotes' }, + quotes: [], + } as unknown as TransactionData), + ).toBe(true); + }); + it('returns true when fiat payment method is selected and no quotes', () => { expect( isQuoteRetryPending({ diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index e7f5897667a..2d58c9161c9 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -322,24 +322,25 @@ function syncTransaction({ } /** - * Determine whether a transaction is waiting on a quote retry: it needs - * quotes — a payment token with pending conversions, or a selected fiat - * payment method — but has none, meaning the last quote load failed or - * returned nothing. + * Determine whether a transaction is waiting on a quote retry: it has no + * quotes even though the last quote load failed, or quotes are needed — a + * payment token with pending conversions, or a selected fiat payment method. * * @param transactionData - Transaction data to check. - * @returns True when the transaction needs quotes but has none. + * @returns True when the transaction has no quotes but should have some. */ export function isQuoteRetryPending( transactionData: TransactionData, ): boolean { - const { fiatPayment, paymentToken, quotes, sourceAmounts } = transactionData; + const { fiatPayment, paymentToken, quoteError, quotes, sourceAmounts } = + transactionData; if (quotes?.length) { return false; } return ( + Boolean(quoteError) || (Boolean(paymentToken) && (sourceAmounts?.length ?? 0) > 0) || Boolean(fiatPayment?.selectedPaymentMethodId) ); From fbefa81e68345ff58834cb608c8f3edd367936ff Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 12:08:21 +0200 Subject: [PATCH 6/8] feat: surface quote load failures only when quotes are needed and none usable --- .../src/utils/quotes.test.ts | 52 ++++++++- .../src/utils/quotes.ts | 110 +++++++++++++----- 2 files changed, 132 insertions(+), 30 deletions(-) diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index 1fbd223cecc..1e23bbe8e11 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -1365,7 +1365,15 @@ describe('Quotes Utils', () => { expect(transactionDataMock.quotes).toStrictEqual([]); }); - it('keeps executable quotes when the quote pipeline throws', async () => { + it('keeps executable quotes and stays silent when a refresh throws', async () => { + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [QUOTE_MOCK], + }, + }, + }); getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); calculateTotalsMock.mockImplementationOnce(() => { throw new Error('calculateTotals failed'); @@ -1382,6 +1390,48 @@ describe('Quotes Utils', () => { ); expect(transactionDataMock.quotes).toStrictEqual([QUOTE_MOCK]); + expect(transactionDataMock.quoteError).toBeUndefined(); + expect(transactionDataMock.quotesLastUpdated).toStrictEqual( + expect.any(Number), + ); + }); + + it('stays silent when a direct route fails', async () => { + const noOpQuote = { + strategy: TransactionPayStrategy.None, + } as TransactionPayQuote; + + getControllerStateMock.mockReturnValue({ + transactionData: { + [TRANSACTION_ID_MOCK]: { + ...cloneDeep(TRANSACTION_DATA_MOCK), + quotes: [noOpQuote], + sourceAmounts: [], + }, + }, + }); + getQuotesMock.mockResolvedValueOnce([QUOTE_MOCK]); + calculateTotalsMock.mockImplementationOnce(() => { + throw new Error('calculateTotals failed'); + }); + + await expect(run()).rejects.toThrow('calculateTotals failed'); + + // Seed the draft with the no-op quote of the direct route. + const transactionDataMock: Record = { + quotes: [noOpQuote], + }; + updateTransactionDataMock.mock.calls.forEach((call) => + call[1](transactionDataMock), + ); + + // Direct routes need no quotes: the no-op marker stays and no error + // is surfaced. + expect(transactionDataMock.quotes).toStrictEqual([noOpQuote]); + expect(transactionDataMock.quoteError).toBeUndefined(); + expect(transactionDataMock.quotesLastUpdated).toStrictEqual( + expect.any(Number), + ); }); it('does not persist quote error when pay state was removed', async () => { diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 2d58c9161c9..cea5b98e12c 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -202,33 +202,12 @@ export async function updateQuotes( return false; } - // Persist the failure so clients can surface it, and stamp the update - // time so the refresh loop retries on the normal interval instead of - // every tick. - updateExistingTransactionData( + persistQuoteLoadFailure({ + error: error as Error, messenger, transactionId, updateTransactionData, - (data) => { - data.quoteError = { - message: (error as Error).message, - reason: 'no-quotes', - }; - data.quotesLastUpdated = Date.now(); - - // Stale no-op quotes from a previous direct route would block the - // retry loop, so drop them. Executable quotes stay usable until a - // refresh replaces them. - if ( - data.quotes?.length && - data.quotes.every( - (quote) => quote.strategy === TransactionPayStrategy.None, - ) - ) { - data.quotes = []; - } - }, - ); + }); throw error; } finally { @@ -332,20 +311,93 @@ function syncTransaction({ export function isQuoteRetryPending( transactionData: TransactionData, ): boolean { - const { fiatPayment, paymentToken, quoteError, quotes, sourceAmounts } = - transactionData; - - if (quotes?.length) { + if (transactionData.quotes?.length) { return false; } + return Boolean(transactionData.quoteError) || needsQuotes(transactionData); +} + +/** + * Determine whether a transaction needs quotes: a payment token with pending + * conversions, or a selected fiat payment method. Direct routes need none — + * their no-op quote is generated locally. + * + * @param transactionData - Transaction data to check. + * @returns True when the transaction needs quotes. + */ +function needsQuotes( + transactionData: TransactionData | Draft, +): boolean { + const { fiatPayment, paymentToken, sourceAmounts } = transactionData; + return ( - Boolean(quoteError) || (Boolean(paymentToken) && (sourceAmounts?.length ?? 0) > 0) || Boolean(fiatPayment?.selectedPaymentMethodId) ); } +/** + * Persist a failed quote load so the refresh loop can retry it. + * + * Always stamps `quotesLastUpdated` so the next attempt waits a full refresh + * interval. Surfaces `quoteError` 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. + * + * @param request - Request object. + * @param request.error - Error thrown by the quote load. + * @param request.messenger - Messenger instance. + * @param request.transactionId - ID of the failed transaction. + * @param request.updateTransactionData - Callback to update transaction data. + */ +function persistQuoteLoadFailure({ + error, + messenger, + transactionId, + updateTransactionData, +}: { + error: Error; + messenger: TransactionPayControllerMessenger; + transactionId: string; + updateTransactionData: UpdateTransactionDataCallback; +}): void { + const currentData = messenger.call('TransactionPayController:getState') + .transactionData[transactionId]; + + // Skip late writes so cleanup-removed entries are not recreated. + if (!currentData) { + return; + } + + const hasExecutableQuotes = Boolean( + currentData.quotes?.some( + (quote) => quote.strategy !== TransactionPayStrategy.None, + ), + ); + + const shouldSurfaceError = !hasExecutableQuotes && needsQuotes(currentData); + + updateTransactionData(transactionId, (data) => { + data.quotesLastUpdated = Date.now(); + + if (!shouldSurfaceError) { + return; + } + + data.quoteError = { + message: error.message, + reason: 'no-quotes', + }; + + if (data.quotes?.length) { + data.quotes = []; + } + }); +} + /** * Refresh quotes for all transactions if expired. * From efdbcabb635c50c82a0df61daa7ce85b35638f94 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 12 Aug 2026 12:14:16 +0200 Subject: [PATCH 7/8] docs: add changelog entries for quote load retry --- packages/transaction-pay-controller/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 28b5678e75a..bb702017080 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/transaction-controller` from `^69.5.1` to `^69.5.2` ([#9823](https://github.com/MetaMask/core/pull/9823)) +### Fixed + +- Retry failed or empty quote loads on the refresh interval, so one failed quote fetch no longer permanently strands a transaction without quotes ([#9837](https://github.com/MetaMask/core/pull/9837)) +- Persist unexpected quote load failures to `quoteError` when quotes are needed and none are usable, instead of silently swallowing them ([#9837](https://github.com/MetaMask/core/pull/9837)) + ## [26.3.0] ### Added From b94dcdd9d7bdda4a6f47f0a535ccebd928d0090c Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:05:08 +0200 Subject: [PATCH 8/8] style: format isQuoteRetryPending signature for oxfmt --- packages/transaction-pay-controller/src/utils/quotes.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index cea5b98e12c..00e36cc0079 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -308,9 +308,7 @@ function syncTransaction({ * @param transactionData - Transaction data to check. * @returns True when the transaction has no quotes but should have some. */ -export function isQuoteRetryPending( - transactionData: TransactionData, -): boolean { +export function isQuoteRetryPending(transactionData: TransactionData): boolean { if (transactionData.quotes?.length) { return false; }