diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 8b1bba13806..2143ccbf44d 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/assets-controller` from `^13.1.2` to `^13.1.3` ([#9873](https://github.com/MetaMask/core/pull/9873)) - 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 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..1e23bbe8e11 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, @@ -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]); @@ -1316,6 +1321,137 @@ 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('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 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'); + }); + + 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]); + 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 () => { + 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[]>(); @@ -1526,6 +1662,340 @@ 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 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: { + [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('stamps the attempt time when the update reports no change', 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, + ); + + // 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('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'; + + 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 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({ + 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..00e36cc0079 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'; @@ -200,6 +201,14 @@ export async function updateQuotes( log('Quote request aborted', { transactionId, reason: signal.reason }); return false; } + + persistQuoteLoadFailure({ + error: error as Error, + messenger, + transactionId, + updateTransactionData, + }); + throw error; } finally { if (!signal.aborted) { @@ -291,9 +300,109 @@ function syncTransaction({ ); } +/** + * 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 has no quotes but should have some. + */ +export function isQuoteRetryPending(transactionData: TransactionData): boolean { + 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(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. * + * 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 +419,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,20 +453,74 @@ 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 }); + } else { + // Not updated (e.g. transaction no longer unapproved): stamp the + // attempt so the next one waits a full interval, not every tick. + 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(messenger, transactionId, updateTransactionData); + log('Failed to refresh quotes', { transactionId, error }); } } } +function stampQuotesLastUpdated( + messenger: TransactionPayControllerMessenger, + transactionId: string, + updateTransactionData: UpdateTransactionDataCallback, +): void { + 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( transactionId: string, ): AbortController {