feat: retry failed quote loads and surface quote errors in TransactionPayController

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

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;

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 });
+      });
     }
   }
 

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<typeof getMessengerMock>['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]),

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();
     }
   }