From 5eddbece043f9ec26895eee3d5fe706efc77d914 Mon Sep 17 00:00:00 2001 From: Julien Fontanel Date: Tue, 20 Oct 2026 17:14:43 +0200 Subject: [PATCH 1/2] feat(tron): use shared fetch policy helpers --- packages/tron-wallet-snap/CHANGELOG.md | 2 +- packages/tron-wallet-snap/jest.config.js | 8 +- .../src/clients/price-api/PriceApiClient.ts | 31 +--- .../SecurityAlertsApiClient.test.ts | 174 ++++++++++++++++++ .../SecurityAlertsApiClient.ts | 37 ++-- .../clients/token-api/TokenApiClient.test.ts | 99 +++++++--- .../src/clients/token-api/TokenApiClient.ts | 30 ++- .../src/clients/tron-http/TronHttpClient.ts | 10 +- .../src/clients/trongrid/TrongridApiClient.ts | 5 +- .../src/clients/trongrid/errors.ts | 3 +- .../src/clients/tronweb/TronWebFactory.ts | 20 +- .../src/utils/fetchPolicy.test.ts | 96 ++++++++++ .../tron-wallet-snap/src/utils/fetchPolicy.ts | 67 ++----- 13 files changed, 431 insertions(+), 151 deletions(-) create mode 100644 packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts create mode 100644 packages/tron-wallet-snap/src/utils/fetchPolicy.test.ts diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index 67ef2e11b..c7c3d244f 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Retry transient TronGrid, Tron provider, price API, and signed transaction broadcast failures with bounded backoff and per-attempt timeouts. Broadcast retries reuse the same signed transaction ID, and duplicate broadcasts are treated as successful. ([#472](https://github.com/MetaMask/internal-snaps/pull/472)) +- Retry transient TronGrid, Tron provider, price API, token API, security alerts API, and signed transaction broadcast failures with bounded backoff and per-attempt timeouts. Broadcast retries reuse the same signed transaction ID, and duplicate broadcasts are treated as successful. ([#472](https://github.com/MetaMask/internal-snaps/pull/472)) ## [4.2.0] diff --git a/packages/tron-wallet-snap/jest.config.js b/packages/tron-wallet-snap/jest.config.js index e7c777273..8d38abcf4 100644 --- a/packages/tron-wallet-snap/jest.config.js +++ b/packages/tron-wallet-snap/jest.config.js @@ -16,10 +16,10 @@ module.exports = { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 73.58, - functions: 83.36, - lines: 87.64, - statements: 87.63, + branches: 74.35, + functions: 83.43, + lines: 88.11, + statements: 88.09, }, }, }; diff --git a/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts b/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts index bf6dacad2..ea62d6635 100644 --- a/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts +++ b/packages/tron-wallet-snap/src/clients/price-api/PriceApiClient.ts @@ -5,9 +5,10 @@ import { UrlStruct, buildUrl, createFetchPolicy, - createFetchPolicyError, + fetchWithPolicy, } from '@metamask/snap-networks-utils'; import type { + FetchPolicy, ICache, Logger, Serializable, @@ -18,10 +19,6 @@ import { mapKeys } from 'lodash'; import { SNAP_OWNED_ASSETS } from '../../constants'; import type { ConfigProvider } from '../../services/config'; -import { - executeWithFetchPolicy, - parseRetryAfter, -} from '../../utils/fetchPolicy'; import logger from '../../utils/logger'; import type { SpotPrice, SpotPrices, VsCurrencyParam } from './types'; import { SpotPricesStruct, VsCurrencyParamStruct } from './types'; @@ -37,7 +34,7 @@ export class PriceApiClient { readonly #cache: ICache; - readonly #policy: ReturnType; + readonly #policy: FetchPolicy; readonly cacheTtlsMilliseconds: { spotPrices: number; @@ -106,25 +103,11 @@ export class PriceApiClient { }, }); - const responseBody = await executeWithFetchPolicy( + const responseBody = await fetchWithPolicy( this.#policy, - async () => { - const result = await this.#fetch(url, { - signal: AbortSignal.timeout(10_000), - }); - if (!result.ok) { - throw createFetchPolicyError( - `HTTP error! status: ${result.status}`, - { - httpStatus: result.status, - retryAfterMs: parseRetryAfter( - result.headers?.get('Retry-After'), - ), - }, - ); - } - return result.text(); - }, + url, + undefined, + this.#fetch, ); const spotPrices = JSON.parse(responseBody); diff --git a/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts b/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts new file mode 100644 index 000000000..076a09dc8 --- /dev/null +++ b/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts @@ -0,0 +1,174 @@ +import type { Logger } from '@metamask/snap-networks-utils'; +import { Types as TronwebTypes } from 'tronweb'; + +import type { ConfigProvider } from '../../services/config'; +import { mockLogger } from '../../utils/mockLogger'; +import { SecurityAlertsApiClient } from './SecurityAlertsApiClient'; +import * as securityAlertsUtils from './utils'; + +const VALID_TRANSACTION_RAW_DATA: TronwebTypes.Transaction['raw_data'] = { + contract: [ + { + type: TronwebTypes.ContractType.TransferContract, + parameter: { + type_url: 'type.googleapis.com/protocol.TransferContract', + value: { + owner_address: `41${'a'.repeat(40)}`, + to_address: `41${'b'.repeat(40)}`, + amount: 1_000_000, + }, + }, + }, + ], + ref_block_bytes: '0000', + ref_block_hash: '0'.repeat(16), + expiration: Date.now() + 60_000, + timestamp: Date.now(), +}; + +const VALID_SCAN_RESPONSE = { + simulation: { + status: 'Success', + }, + validation: { + status: 'Success', + result_type: 'Benign', + }, +}; + +describe('SecurityAlertsApiClient', () => { + const configProvider = { + config: { + securityAlertsApi: { + baseUrl: 'https://security-alerts.example.com', + }, + }, + } as ConfigProvider; + + it('scans a supported transaction through the fetch policy', async () => { + const fetchMock = jest.fn().mockResolvedValueOnce({ + ok: true, + text: jest + .fn() + .mockResolvedValueOnce(JSON.stringify(VALID_SCAN_RESPONSE)), + }); + const client = new SecurityAlertsApiClient( + configProvider, + mockLogger as unknown as Logger, + fetchMock as unknown as typeof globalThis.fetch, + ); + + const result = await client.scanTransaction({ + accountAddress: 'TExvJsxzPyAZ2NtkrWgNKnbLkpqnFJ73DT', + transactionRawData: VALID_TRANSACTION_RAW_DATA, + origin: 'https://example.com', + }); + + expect(result).toStrictEqual(VALID_SCAN_RESPONSE); + expect(fetchMock).toHaveBeenCalledWith( + 'https://security-alerts.example.com/tron/transaction/scan', + expect.objectContaining({ + method: 'POST', + signal: expect.any(AbortSignal), + }), + ); + }); + + it('retries transient HTTP failures and does not retry 400 responses', async () => { + const fetchMock = jest + .fn() + .mockResolvedValueOnce({ + ok: false, + status: 503, + headers: { get: () => null }, + }) + .mockResolvedValueOnce({ + ok: true, + text: jest + .fn() + .mockResolvedValueOnce(JSON.stringify(VALID_SCAN_RESPONSE)), + }); + const client = new SecurityAlertsApiClient( + configProvider, + mockLogger as unknown as Logger, + fetchMock as unknown as typeof globalThis.fetch, + ); + + expect( + await client.scanTransaction({ + accountAddress: 'TExvJsxzPyAZ2NtkrWgNKnbLkpqnFJ73DT', + transactionRawData: VALID_TRANSACTION_RAW_DATA, + origin: 'https://example.com', + }), + ).toStrictEqual(VALID_SCAN_RESPONSE); + expect(fetchMock).toHaveBeenCalledTimes(2); + + fetchMock.mockClear(); + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 400, + headers: { get: () => null }, + }); + + await expect( + client.scanTransaction({ + accountAddress: 'TExvJsxzPyAZ2NtkrWgNKnbLkpqnFJ73DT', + transactionRawData: VALID_TRANSACTION_RAW_DATA, + origin: 'https://example.com', + }), + ).rejects.toThrow('HTTP error! status: 400'); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('throws when the transaction is not supported for scanning', async () => { + const fetchMock = jest.fn(); + const client = new SecurityAlertsApiClient( + configProvider, + mockLogger as unknown as Logger, + fetchMock as unknown as typeof globalThis.fetch, + ); + + await expect( + client.scanTransaction({ + accountAddress: 'TExvJsxzPyAZ2NtkrWgNKnbLkpqnFJ73DT', + transactionRawData: { + ...VALID_TRANSACTION_RAW_DATA, + contract: [], + }, + origin: 'https://example.com', + }), + ).rejects.toThrow('Transaction is not supported for scanning.'); + expect(fetchMock).not.toHaveBeenCalled(); + expect( + SecurityAlertsApiClient.isContractTypeSupported({ + ...VALID_TRANSACTION_RAW_DATA, + contract: [], + }), + ).toBe(false); + }); + + it('throws when scan parameters cannot be extracted from the transaction', async () => { + const extractSpy = jest + .spyOn(securityAlertsUtils, 'extractScanParametersFromTransactionData') + .mockReturnValueOnce(null); + const fetchMock = jest.fn(); + const client = new SecurityAlertsApiClient( + configProvider, + mockLogger as unknown as Logger, + fetchMock as unknown as typeof globalThis.fetch, + ); + + try { + await expect( + client.scanTransaction({ + accountAddress: 'TExvJsxzPyAZ2NtkrWgNKnbLkpqnFJ73DT', + transactionRawData: VALID_TRANSACTION_RAW_DATA, + origin: 'https://example.com', + }), + ).rejects.toThrow('Could not extract scan parameters from transaction.'); + expect(fetchMock).not.toHaveBeenCalled(); + } finally { + extractSpy.mockRestore(); + } + }); +}); diff --git a/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.ts b/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.ts index 3de81216a..b05e70d6b 100644 --- a/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.ts +++ b/packages/tron-wallet-snap/src/clients/security-alerts-api/SecurityAlertsApiClient.ts @@ -1,6 +1,10 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import type { Logger } from '@metamask/snap-networks-utils'; +import type { FetchPolicy, Logger } from '@metamask/snap-networks-utils'; +import { + createFetchPolicy, + fetchWithPolicy, +} from '@metamask/snap-networks-utils'; import { assert } from '@metamask/superstruct'; import { Types as TronwebTypes } from 'tronweb'; @@ -54,16 +58,27 @@ export class SecurityAlertsApiClient { readonly #baseUrl: string; + readonly #policy: FetchPolicy; + /** * Creates a new SecurityAlertsApiClient instance. * * @param configProvider - The configuration provider. * @param logger - Logger instance for logging. + * @param _fetch - The fetch implementation, defaulting to `globalThis.fetch`. */ - constructor(configProvider: ConfigProvider, logger: Logger) { - this.#fetch = fetch; + constructor( + configProvider: ConfigProvider, + logger: Logger, + _fetch: typeof globalThis.fetch = globalThis.fetch, + ) { + this.#fetch = _fetch; this.#logger = logger.withPrefix('[🔒 SecurityAlertsApiClient]'); this.#baseUrl = configProvider.config.securityAlertsApi.baseUrl; + this.#policy = createFetchPolicy({ + service: 'security_alerts_api', + logger: this.#logger, + }); } /** @@ -108,7 +123,8 @@ export class SecurityAlertsApiClient { throw new Error('Could not extract scan parameters from transaction.'); } - const response = await this.#fetch( + const body = await fetchWithPolicy( + this.#policy, `${this.#baseUrl}/tron/transaction/scan`, { headers, @@ -122,19 +138,10 @@ export class SecurityAlertsApiClient { options, }), }, + this.#fetch, ); - if (!response.ok) { - const errorBody = await response.text(); - this.#logger.error( - `Security Alerts API error: ${response.status} - ${errorBody}`, - ); - throw new Error( - `Security Alerts API error: ${response.status} - ${errorBody}`, - ); - } - - const data = await response.json(); + const data = JSON.parse(body); assert(data, SecurityAlertResponseStruct); return data; diff --git a/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.test.ts b/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.test.ts index d0ef20450..18ea0a810 100644 --- a/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.test.ts +++ b/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.test.ts @@ -90,7 +90,9 @@ describe('TokenApiClient', () => { ]; mockFetch.mockResolvedValueOnce({ ok: true, - json: jest.fn().mockResolvedValueOnce(MOCK_METADATA_RESPONSE), + text: jest + .fn() + .mockResolvedValueOnce(JSON.stringify(MOCK_METADATA_RESPONSE)), }); const metadata = await client.getTokensMetadata(tokenAddresses); @@ -140,7 +142,9 @@ describe('TokenApiClient', () => { mockFetch.mockResolvedValue({ ok: true, - json: jest.fn().mockResolvedValue(MOCK_METADATA_RESPONSE), + text: jest + .fn() + .mockResolvedValue(JSON.stringify(MOCK_METADATA_RESPONSE)), }); await client.getTokensMetadata(tokenAddresses); @@ -185,18 +189,22 @@ describe('TokenApiClient', () => { mockFetch .mockResolvedValueOnce({ ok: true, - json: jest.fn().mockResolvedValueOnce(MOCK_METADATA_RESPONSE), + text: jest + .fn() + .mockResolvedValueOnce(JSON.stringify(MOCK_METADATA_RESPONSE)), }) .mockResolvedValueOnce({ ok: true, - json: jest.fn().mockResolvedValueOnce([ - { - decimals: 6, - assetId: thirdTokenAddress, - name: 'Third token', - symbol: 'THIRD', - }, - ]), + text: jest.fn().mockResolvedValueOnce( + JSON.stringify([ + { + decimals: 6, + assetId: thirdTokenAddress, + name: 'Third token', + symbol: 'THIRD', + }, + ]), + ), }); const metadata = await client.getTokensMetadata(tokenAddresses); @@ -242,18 +250,61 @@ describe('TokenApiClient', () => { ]; const errorMessage = 'Error fetching token metadata'; - mockFetch.mockRejectedValueOnce(new Error(errorMessage)); + mockFetch.mockRejectedValue(new Error(errorMessage)); await expect(client.getTokensMetadata(tokenAddresses)).rejects.toThrow( errorMessage, ); + expect(mockFetch).toHaveBeenCalledTimes(4); expect(logger.error).toHaveBeenCalledWith( - new Error(errorMessage), + expect.objectContaining({ message: errorMessage }), errorMessage, ); }); }); + it('retries transient HTTP errors and does not retry 404 responses', async () => { + await withTokenApiClient(async ({ client, mockFetch }) => { + const tokenAddresses = [ + `${ + Networks[Network.Mainnet].caip2Id + }/trc20:TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t` as TokenCaipAssetType, + ]; + mockFetch + .mockResolvedValueOnce({ + ok: false, + status: 503, + headers: { get: () => null }, + }) + .mockResolvedValueOnce({ + ok: true, + text: jest + .fn() + .mockResolvedValueOnce( + JSON.stringify([MOCK_METADATA_RESPONSE[0]]), + ), + }); + + const metadata = await client.getTokensMetadata(tokenAddresses); + + const firstTokenAddress = tokenAddresses[0] as TokenCaipAssetType; + expect(metadata[firstTokenAddress]).toBeDefined(); + expect(mockFetch).toHaveBeenCalledTimes(2); + + mockFetch.mockClear(); + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + headers: { get: () => null }, + }); + + await expect(client.getTokensMetadata(tokenAddresses)).rejects.toThrow( + 'HTTP error! status: 404', + ); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + }); + it('throws an error if the response includes an invalid assetId', async () => { await withTokenApiClient(async ({ client, mockFetch }) => { const tokenAddresses = [ @@ -267,14 +318,16 @@ describe('TokenApiClient', () => { mockFetch.mockResolvedValueOnce({ ok: true, - json: jest.fn().mockResolvedValueOnce([ - { - decimals: 9, - assetId: 'bad-asset-id', - name: 'Popcat 1', - symbol: 'POPCAT', - }, - ]), + text: jest.fn().mockResolvedValueOnce( + JSON.stringify([ + { + decimals: 9, + assetId: 'bad-asset-id', + name: 'Popcat 1', + symbol: 'POPCAT', + }, + ]), + ), }); await expect(client.getTokensMetadata(tokenAddresses)).rejects.toThrow( @@ -295,7 +348,9 @@ describe('TokenApiClient', () => { const tokenAddresses = [supportedAssetType, unsupportedAssetType]; mockFetch.mockResolvedValueOnce({ ok: true, - json: jest.fn().mockResolvedValueOnce([MOCK_METADATA_RESPONSE[0]]), + text: jest + .fn() + .mockResolvedValueOnce(JSON.stringify([MOCK_METADATA_RESPONSE[0]])), }); const metadata = await client.getTokensMetadata(tokenAddresses); diff --git a/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts b/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts index ded02ac3e..72a53644d 100644 --- a/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts +++ b/packages/tron-wallet-snap/src/clients/token-api/TokenApiClient.ts @@ -1,5 +1,11 @@ -import { UrlStruct, buildUrl, chunks } from '@metamask/snap-networks-utils'; -import type { Logger } from '@metamask/snap-networks-utils'; +import { + createFetchPolicy, + fetchWithPolicy, + UrlStruct, + buildUrl, + chunks, +} from '@metamask/snap-networks-utils'; +import type { FetchPolicy, Logger } from '@metamask/snap-networks-utils'; import type { FungibleAssetMetadata } from '@metamask/snaps-sdk'; import { array, assert } from '@metamask/superstruct'; import type { Infer } from '@metamask/superstruct'; @@ -34,6 +40,8 @@ export class TokenApiClient { readonly #tokenIconBaseUrl: string; + readonly #policy: FetchPolicy; + public static readonly supportedNetworks = [ Network.Mainnet, Network.Nile, @@ -56,6 +64,10 @@ export class TokenApiClient { this.#baseUrl = baseUrl; this.#chunkSize = chunkSize; this.#tokenIconBaseUrl = staticApi.baseUrl; + this.#policy = createFetchPolicy({ + service: 'token_api', + logger: this.#logger, + }); } async #fetchTokenMetadataBatch( @@ -71,13 +83,13 @@ export class TokenApiClient { }, }); - const response = await this.#fetch(url); - - if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - - const data = await response.json(); + const body = await fetchWithPolicy( + this.#policy, + url, + undefined, + this.#fetch, + ); + const data = JSON.parse(body); assert(data, TokenMetadataResponseStruct); diff --git a/packages/tron-wallet-snap/src/clients/tron-http/TronHttpClient.ts b/packages/tron-wallet-snap/src/clients/tron-http/TronHttpClient.ts index 9ff9bb287..757814ace 100644 --- a/packages/tron-wallet-snap/src/clients/tron-http/TronHttpClient.ts +++ b/packages/tron-wallet-snap/src/clients/tron-http/TronHttpClient.ts @@ -2,16 +2,14 @@ import { buildUrl, createFetchPolicy, createFetchPolicyError, + executeWithFetchPolicy, + parseRetryAfter, } from '@metamask/snap-networks-utils'; -import type { Logger } from '@metamask/snap-networks-utils'; +import type { FetchPolicy, Logger } from '@metamask/snap-networks-utils'; import { assert } from '@metamask/superstruct'; import { Network } from '../../constants'; import type { ConfigProvider } from '../../services/config'; -import { - executeWithFetchPolicy, - parseRetryAfter, -} from '../../utils/fetchPolicy'; import { hexToString } from '../../utils/hex'; import logger from '../../utils/logger'; import { @@ -49,7 +47,7 @@ export class TronHttpClient { } > = new Map(); - readonly #policies: Map>; + readonly #policies: Map; constructor({ configProvider, diff --git a/packages/tron-wallet-snap/src/clients/trongrid/TrongridApiClient.ts b/packages/tron-wallet-snap/src/clients/trongrid/TrongridApiClient.ts index 6415c26d2..501214e1d 100644 --- a/packages/tron-wallet-snap/src/clients/trongrid/TrongridApiClient.ts +++ b/packages/tron-wallet-snap/src/clients/trongrid/TrongridApiClient.ts @@ -1,9 +1,11 @@ import { createFetchPolicy, + executeWithFetchPolicy, useCacheUntil, buildUrl, } from '@metamask/snap-networks-utils'; import type { + FetchPolicy, ICache, Logger, ResultWithExpiry, @@ -13,7 +15,6 @@ import { assert } from '@metamask/superstruct'; import { Network } from '../../constants'; import type { ConfigProvider } from '../../services/config'; -import { executeWithFetchPolicy } from '../../utils/fetchPolicy'; import logger from '../../utils/logger'; import type { TronHttpClient } from '../tron-http/TronHttpClient'; import type { ChainParameter } from '../tron-http/types'; @@ -67,7 +68,7 @@ export class TrongridApiClient { readonly #cache: ICache; - readonly #policies: Map>; + readonly #policies: Map; /** * Cached version of getChainParameters that uses maintenance-aligned expiry. diff --git a/packages/tron-wallet-snap/src/clients/trongrid/errors.ts b/packages/tron-wallet-snap/src/clients/trongrid/errors.ts index 1f7e40e66..1e98f1af5 100644 --- a/packages/tron-wallet-snap/src/clients/trongrid/errors.ts +++ b/packages/tron-wallet-snap/src/clients/trongrid/errors.ts @@ -1,6 +1,5 @@ import type { FetchPolicyError } from '@metamask/snap-networks-utils'; - -import { parseRetryAfter } from '../../utils/fetchPolicy'; +import { parseRetryAfter } from '@metamask/snap-networks-utils'; /** * Error thrown when a TronGrid HTTP request fails with a non-2xx status. diff --git a/packages/tron-wallet-snap/src/clients/tronweb/TronWebFactory.ts b/packages/tron-wallet-snap/src/clients/tronweb/TronWebFactory.ts index c8a52dff4..8594f210c 100644 --- a/packages/tron-wallet-snap/src/clients/tronweb/TronWebFactory.ts +++ b/packages/tron-wallet-snap/src/clients/tronweb/TronWebFactory.ts @@ -1,10 +1,13 @@ -import { createFetchPolicy } from '@metamask/snap-networks-utils'; -import type { Logger } from '@metamask/snap-networks-utils'; +import { + createFetchPolicy, + executeWithFetchPolicy, +} from '@metamask/snap-networks-utils'; +import type { FetchPolicy, Logger } from '@metamask/snap-networks-utils'; import { providers, TronWeb } from 'tronweb'; import { Network } from '../../constants'; import type { ConfigProvider } from '../../services/config'; -import { executeWithFetchPolicy } from '../../utils/fetchPolicy'; +import { toFetchPolicyError } from '../../utils/fetchPolicy'; import logger from '../../utils/logger'; const REQUEST_TIMEOUT_MS = 10_000; @@ -12,12 +15,9 @@ const REQUEST_TIMEOUT_MS = 10_000; export class TronWebFactory { readonly #configProvider: ConfigProvider; - readonly #policies: Map>; + readonly #policies: Map; - readonly #broadcastPolicies: Map< - Network, - ReturnType - >; + readonly #broadcastPolicies: Map; constructor({ configProvider, @@ -85,7 +85,7 @@ export class TronWebFactory { if (!policy) { throw new Error(`No broadcast policy configured for network: ${network}`); } - return executeWithFetchPolicy(policy, operation); + return executeWithFetchPolicy(policy, operation, toFetchPolicyError); } async #execute( @@ -96,7 +96,7 @@ export class TronWebFactory { if (!policy) { throw new Error(`No policy configured for network: ${network}`); } - return executeWithFetchPolicy(policy, operation); + return executeWithFetchPolicy(policy, operation, toFetchPolicyError); } /** diff --git a/packages/tron-wallet-snap/src/utils/fetchPolicy.test.ts b/packages/tron-wallet-snap/src/utils/fetchPolicy.test.ts new file mode 100644 index 000000000..d76bb37f2 --- /dev/null +++ b/packages/tron-wallet-snap/src/utils/fetchPolicy.test.ts @@ -0,0 +1,96 @@ +import { + createFetchPolicy, + createFetchPolicyError, + executeWithFetchPolicy, + FetchPolicyErrorName, +} from '@metamask/snap-networks-utils'; + +import { toFetchPolicyError } from './fetchPolicy'; +import { mockLogger } from './mockLogger'; + +describe('toFetchPolicyError', () => { + it('classifies non-errors as network failures', () => { + const error = toFetchPolicyError('boom'); + + expect(error.name).toBe(FetchPolicyErrorName.Network); + expect(error.message).toBe('Network request failed'); + expect(error.cause).toBe('boom'); + }); + + it('preserves policy-shaped HTTP errors', () => { + const error = Object.assign(new Error('Bad request'), { httpStatus: 400 }); + + expect(toFetchPolicyError(error)).toBe(error); + }); + + it('preserves policy network errors', () => { + const error = createFetchPolicyError('Network failed', { + name: FetchPolicyErrorName.Network, + }); + + expect(toFetchPolicyError(error)).toBe(error); + }); + + it('preserves policy timeouts', () => { + const error = createFetchPolicyError('Timed out', { + name: FetchPolicyErrorName.Timeout, + }); + + expect(toFetchPolicyError(error)).toBe(error); + }); + + it('extracts status and Retry-After from axios-style responses', () => { + const error = Object.assign(new Error('Too Many Requests'), { + response: { + status: 429, + headers: { 'retry-after': '5' }, + }, + }); + + expect(toFetchPolicyError(error)).toMatchObject({ + httpStatus: 429, + retryAfterMs: 5000, + cause: error, + }); + }); + + it('maps axios ECONNABORTED errors to timeouts', () => { + const error = Object.assign(new Error('timeout of 10000ms exceeded'), { + code: 'ECONNABORTED', + }); + + const mapped = toFetchPolicyError(error); + + expect(mapped.name).toBe(FetchPolicyErrorName.Timeout); + expect(mapped.cause).toBe(error); + }); + + it('classifies other errors as network failures', () => { + const error = new TypeError('fetch failed'); + + const mapped = toFetchPolicyError(error); + + expect(mapped.name).toBe(FetchPolicyErrorName.Network); + expect(mapped.cause).toBe(error); + }); + + it('retries axios-style transient errors through executeWithFetchPolicy', async () => { + const policy = createFetchPolicy({ + service: 'test', + logger: mockLogger, + }); + const operation = jest + .fn, []>() + .mockRejectedValueOnce( + Object.assign(new Error('timeout of 10000ms exceeded'), { + code: 'ECONNABORTED', + }), + ) + .mockResolvedValue('ok'); + + expect( + await executeWithFetchPolicy(policy, operation, toFetchPolicyError), + ).toBe('ok'); + expect(operation).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/tron-wallet-snap/src/utils/fetchPolicy.ts b/packages/tron-wallet-snap/src/utils/fetchPolicy.ts index d638882d6..d167e0514 100644 --- a/packages/tron-wallet-snap/src/utils/fetchPolicy.ts +++ b/packages/tron-wallet-snap/src/utils/fetchPolicy.ts @@ -1,12 +1,9 @@ +import type { FetchPolicyError } from '@metamask/snap-networks-utils'; import { createFetchPolicyError, FetchPolicyErrorName, + parseRetryAfter, } from '@metamask/snap-networks-utils'; -import type { FetchPolicyError } from '@metamask/snap-networks-utils'; - -export type FetchPolicy = { - execute(operation: () => Promise): Promise; -}; type AxiosError = Error & { code?: string; @@ -16,26 +13,6 @@ type AxiosError = Error & { }; }; -/** - * Parse a Retry-After header as milliseconds. - * - * @param value - The header value. - * @returns The delay in milliseconds, if valid. - */ -export const parseRetryAfter = (value: unknown): number | undefined => { - if (typeof value !== 'string') { - return undefined; - } - - const seconds = Number(value); - if (Number.isFinite(seconds) && seconds >= 0) { - return seconds * 1000; - } - - const date = Date.parse(value); - return Number.isNaN(date) ? undefined : Math.max(0, date - Date.now()); -}; - /** * Convert fetch and axios-style failures to the shared fetch-policy shape. * @@ -59,6 +36,14 @@ export const toFetchPolicyError = (error: unknown): FetchPolicyError => { } const axiosError = error as AxiosError; + if ( + (axiosError as FetchPolicyError).httpStatus !== undefined || + error.name === FetchPolicyErrorName.Network || + error.name === FetchPolicyErrorName.Timeout + ) { + return axiosError; + } + const httpStatus = axiosError.response?.status; if (httpStatus !== undefined) { return createFetchPolicyError(error.message, { @@ -70,10 +55,7 @@ export const toFetchPolicyError = (error: unknown): FetchPolicyError => { }); } - if ( - error.name === FetchPolicyErrorName.Timeout || - axiosError.code === 'ECONNABORTED' - ) { + if (axiosError.code === 'ECONNABORTED') { return createFetchPolicyError(error.message, { name: FetchPolicyErrorName.Timeout, cause: error, @@ -85,30 +67,3 @@ export const toFetchPolicyError = (error: unknown): FetchPolicyError => { cause: error, }); }; - -/** - * Execute a network operation through a policy, mapping transport failures. - * - * @param policy - The shared fetch policy. - * @param operation - The network operation. - * @returns The operation result. - */ -export const executeWithFetchPolicy = async ( - policy: FetchPolicy, - operation: () => Promise, -): Promise => - policy.execute(async () => { - try { - return await operation(); - } catch (error) { - if ( - error instanceof Error && - ((error as FetchPolicyError).httpStatus !== undefined || - error.name === FetchPolicyErrorName.Network || - error.name === FetchPolicyErrorName.Timeout) - ) { - throw error; - } - throw toFetchPolicyError(error); - } - }); From fea83f4227442ebc469142d87c8c0faa33e1aa6f Mon Sep 17 00:00:00 2001 From: Julien Fontanel Date: Wed, 21 Oct 2026 10:34:22 +0200 Subject: [PATCH 2/2] chore: fix main rebase issues --- packages/snap-networks-utils/CHANGELOG.md | 2 +- .../SecurityAlertsApiClient.test.ts | 95 +++++++------- .../SecurityAlertsApiClient.ts | 118 ++++++------------ packages/tron-wallet-snap/src/context.ts | 1 - 4 files changed, 79 insertions(+), 137 deletions(-) diff --git a/packages/snap-networks-utils/CHANGELOG.md b/packages/snap-networks-utils/CHANGELOG.md index 5c077017b..ea741d873 100644 --- a/packages/snap-networks-utils/CHANGELOG.md +++ b/packages/snap-networks-utils/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add the `FetchPolicy` type, the minimal policy surface `executeWithFetchPolicy` and `fetchWithPolicy` accept ([#478](https://github.com/MetaMask/internal-snaps/pull/478)) - Add the shared `BaseTransactionScanService` base class for network snap transaction scan services, owning the shared scan dependencies (Security Alerts API client, analytics, logger, error reporting) and the scan analytics orchestration ([#397](https://github.com/MetaMask/internal-snaps/pull/397)) - Add the shared Security Alerts API HTTP client for `BaseTransactionScanService`. -- Add an optional `policy` and `logger` to the shared Security Alerts API client options, retrying transient scan failures (network errors, timeouts, HTTP 408, 425, 429, 500, 502, 503 and 504) with backoff and circuit breaking when given ([#492](https://github.com/MetaMask/internal-snaps/pull/492)) +- Add an optional `policy` to the shared Security Alerts API client options, retrying transient scan failures (network errors, timeouts, HTTP 408, 425, 429, 500, 502, 503 and 504) with backoff and circuit breaking when given ([#492](https://github.com/MetaMask/internal-snaps/pull/492)) ## [3.0.0] diff --git a/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts b/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts index 8e5c65b70..59ccf2723 100644 --- a/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts +++ b/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.test.ts @@ -17,7 +17,6 @@ describe('SecurityAlertsApiClient', () => { ok: true, status: 200, statusText: '', - headers: { get: () => null }, json: jest.fn().mockResolvedValue({ verdict: 'Benign' }), ...overrides, }) as unknown as Response; @@ -77,16 +76,6 @@ describe('SecurityAlertsApiClient', () => { ); }); - it('throws when a policy is given without a logger', () => { - expect( - () => - new SecurityAlertsApiClient({ - scanUrl: SCAN_URL, - policy: { execute: jest.fn() }, - }), - ).toThrow('A logger is required when a fetch policy is given.'); - }); - it('posts the body to the scan URL with the shared headers', async () => { fetchMock.mockResolvedValue(responseWith()); @@ -126,13 +115,30 @@ describe('SecurityAlertsApiClient', () => { .mockRejectedValueOnce(new TypeError('fetch failed')) .mockResolvedValue(responseWith()); - expect(await scan({ policy, logger })).toStrictEqual({ + expect(await scan({ policy })).toStrictEqual({ verdict: 'Benign', }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(logger.warn).toHaveBeenCalled(); }); + it('wraps exhausted network failures in an HttpError when a policy is provided', async () => { + const { policy } = createPolicy(); + const fetchFailure = new TypeError('fetch failed', { + cause: new Error('ECONNREFUSED'), + }); + fetchMock.mockRejectedValue(fetchFailure); + + const error = await caughtError(scan({ policy })); + + expect(error).toBeInstanceOf(HttpError); + expect(error).toMatchObject({ + name: 'HttpError', + message: 'Security Alerts API error', + cause: fetchFailure, + }); + }); + it('retries transient 503 responses through the policy when one is provided', async () => { const { policy, logger } = createPolicy(); fetchMock @@ -141,7 +147,7 @@ describe('SecurityAlertsApiClient', () => { ) .mockResolvedValue(responseWith()); - expect(await scan({ policy, logger })).toStrictEqual({ + expect(await scan({ policy })).toStrictEqual({ verdict: 'Benign', }); expect(fetchMock).toHaveBeenCalledTimes(2); @@ -149,46 +155,47 @@ describe('SecurityAlertsApiClient', () => { }); it('does not retry 400 responses through the policy when one is provided', async () => { - const { policy, logger } = createPolicy(); + const { policy } = createPolicy(); fetchMock.mockResolvedValue( responseWith({ ok: false, status: 400, statusText: '' }), ); - expect(await caughtError(scan({ policy, logger }))).toBeInstanceOf( - HttpError, - ); - expect( - (await caughtError(scan({ policy, logger }))) as Error, - ).toMatchObject({ message: 'HTTP error! status: 400' }); - expect(fetchMock).toHaveBeenCalledTimes(2); + const error = await caughtError(scan({ policy })); + + expect(error).toBeInstanceOf(HttpError); + expect(error).toMatchObject({ message: 'HTTP error! status: 400' }); + expect(fetchMock).toHaveBeenCalledTimes(1); }); - it('retries timeouts mapped from the abort cause through the policy when one is provided', async () => { - const { policy, logger } = createPolicy(); + it('retries timeouts through the policy when one is provided', async () => { + const { policy } = createPolicy(); const timeout = Object.assign(new Error('The operation was aborted'), { name: 'TimeoutError', }); fetchMock.mockRejectedValueOnce(timeout).mockResolvedValue(responseWith()); - expect(await scan({ policy, logger })).toStrictEqual({ + expect(await scan({ policy })).toStrictEqual({ verdict: 'Benign', }); expect(fetchMock).toHaveBeenCalledTimes(2); }); - it('does not retry malformed-response errors through the policy when one is provided', async () => { + it('does not retry malformed JSON responses through the policy when one is provided', async () => { const { policy, logger } = createPolicy(); fetchMock - .mockRejectedValueOnce( - new HttpError('Security Alerts API error', { - cause: new SyntaxError('Unexpected end of JSON input'), + .mockResolvedValueOnce( + responseWith({ + json: jest + .fn() + .mockRejectedValue(new SyntaxError('Unexpected end of JSON input')), }), ) .mockResolvedValue(responseWith()); - expect(await caughtError(scan({ policy, logger }))).toBeInstanceOf( - HttpError, - ); + const error = await caughtError(scan({ policy })); + + expect(error).toBeInstanceOf(HttpError); + expect((error as Error).cause).toBeInstanceOf(SyntaxError); expect(fetchMock).toHaveBeenCalledTimes(1); expect(logger.warn).not.toHaveBeenCalled(); }); @@ -197,33 +204,17 @@ describe('SecurityAlertsApiClient', () => { const { policy, logger } = createPolicy(); fetchMock .mockRejectedValueOnce( - new HttpError('Security Alerts API error', { - cause: Object.assign(new Error('The operation was aborted'), { - name: 'AbortError', - }), + Object.assign(new Error('The operation was aborted'), { + name: 'AbortError', }), ) .mockResolvedValue(responseWith()); - expect(await caughtError(scan({ policy, logger }))).toBeInstanceOf( - HttpError, - ); - expect(fetchMock).toHaveBeenCalledTimes(1); - expect(logger.warn).not.toHaveBeenCalled(); - }); + const error = await caughtError(scan({ policy })); - it('does not retry errors with an unrecognizable cause through the policy', async () => { - const { policy, logger } = createPolicy(); - fetchMock - .mockRejectedValueOnce( - new HttpError('Security Alerts API error', { cause: 'string failure' }), - ) - .mockResolvedValue(responseWith()); - - expect(await caughtError(scan({ policy, logger }))).toBeInstanceOf( - HttpError, - ); + expect(error).toBeInstanceOf(HttpError); expect(fetchMock).toHaveBeenCalledTimes(1); + expect(logger.warn).not.toHaveBeenCalled(); }); it('returns the parsed response body', async () => { diff --git a/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.ts b/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.ts index 71a6fe9d1..38c7c9802 100644 --- a/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.ts +++ b/packages/snap-networks-utils/src/clients/security-alerts-api/SecurityAlertsApiClient.ts @@ -1,17 +1,12 @@ import { assert } from '@metamask/superstruct'; import { HttpError } from '../../utils/errors'; -import type { FetchPolicyError } from '../../utils/fetchPolicy/createFetchPolicy'; -import { - createFetchPolicyError, - FetchPolicyErrorName, -} from '../../utils/fetchPolicy/createFetchPolicy'; +import { FetchPolicyErrorName } from '../../utils/fetchPolicy/createFetchPolicy'; import type { FetchPolicy } from '../../utils/fetchPolicy/executeWithFetchPolicy'; import { executeWithFetchPolicy, parseRetryAfter, } from '../../utils/fetchPolicy/executeWithFetchPolicy'; -import type { Logger } from '../../utils/logger/Logger'; import { UrlStruct } from '../../utils/urlStruct/urlStruct'; /** @@ -43,45 +38,8 @@ export type SecurityAlertsApiClientOptions = { * request is made directly. */ policy?: FetchPolicy; - /** - * Logger for the fetch policy's retries and circuit breaks. Required when a - * policy is given. - */ - logger?: Pick; }; -/** - * Map an {@link HttpError} thrown by the request to the fetch-policy shape. - * - * Non-2xx errors already carry an `httpStatus` (attached when thrown) and pass - * through before this mapper runs. The remaining errors are classified by - * their cause: timeouts and network failures are transient, - * malformed-response and abort causes are not. - * - * @param error - The error to map. - * @returns The error in the fetch-policy shape. - */ -function mapScanError(error: unknown): FetchPolicyError { - const httpError = error as HttpError; - const { cause } = error as Error & { cause?: unknown }; - if (cause instanceof Error) { - if (cause.name === 'TimeoutError') { - return createFetchPolicyError(httpError.message, { - name: FetchPolicyErrorName.Timeout, - cause, - }); - } - if (cause.name !== 'AbortError' && !(cause instanceof SyntaxError)) { - return createFetchPolicyError(httpError.message, { - name: FetchPolicyErrorName.Network, - cause, - }); - } - } - - return httpError; -} - /** * HTTP client for a Security Alerts API scan endpoint. * @@ -111,18 +69,13 @@ export class SecurityAlertsApiClient { * `globalThis.fetch`. * @param options.policy - Optional fetch policy for retries and circuit * breaking. - * @param options.logger - Logger for the policy; required with a policy. */ constructor({ scanUrl, fetch: fetchFn = globalThis.fetch, policy, - logger, }: SecurityAlertsApiClientOptions) { assert(scanUrl, UrlStruct); - if (policy && !logger) { - throw new Error('A logger is required when a fetch policy is given.'); - } this.#fetch = fetchFn; this.#scanUrl = scanUrl; @@ -134,49 +87,48 @@ export class SecurityAlertsApiClient { * * @param body - The chain-specific scan request body. * @returns The parsed scan response. - * @throws HttpError if the request fails, the API responds with a non-2xx - * status, or the response body cannot be parsed as JSON. With a policy, - * throws a fetch-policy error instead when the retries are exhausted or the - * circuit is open. + * @throws HttpError if the request fails, the API responds + * with a non-2xx status, or the response body cannot be parsed as JSON. */ async scanTransaction(body: unknown): Promise { const request = async (): Promise => { - try { - const response = await this.#fetch(this.#scanUrl, { - headers: { ...SECURITY_ALERTS_REQUEST_HEADERS }, - method: 'POST', - body: JSON.stringify(body), - signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), - }); + const response = await this.#fetch(this.#scanUrl, { + headers: { ...SECURITY_ALERTS_REQUEST_HEADERS }, + method: 'POST', + body: JSON.stringify(body), + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); - if (!response.ok) { - throw Object.assign( - new HttpError(`HTTP error! status: ${response.status}`), - { - httpStatus: response.status, - retryAfterMs: parseRetryAfter( - response.headers.get('Retry-After'), - ), - }, - ); - } + if (!response.ok) { + throw Object.assign( + new HttpError(`HTTP error! status: ${response.status}`), + { + httpStatus: response.status, + retryAfterMs: parseRetryAfter(response.headers?.get('Retry-After')), + }, + ); + } - return await response.json(); - } catch (error) { - if (error instanceof HttpError) { - throw error; - } + return await response.json(); + }; - throw new HttpError('Security Alerts API error', { - cause: error, - }); + try { + return await (this.#policy + ? executeWithFetchPolicy(this.#policy, request) + : request()); + } catch (error) { + if (error instanceof HttpError) { + throw error; } - }; - if (!this.#policy) { - return request(); + throw new HttpError('Security Alerts API error', { + cause: + error instanceof Error && + error.name === FetchPolicyErrorName.Network && + error.cause !== undefined + ? error.cause + : error, + }); } - - return executeWithFetchPolicy(this.#policy, request, mapScanError); } } diff --git a/packages/tron-wallet-snap/src/context.ts b/packages/tron-wallet-snap/src/context.ts index 4f413c418..f31ec027b 100644 --- a/packages/tron-wallet-snap/src/context.ts +++ b/packages/tron-wallet-snap/src/context.ts @@ -120,7 +120,6 @@ const securityAlertsHttpClient = new SecurityAlertsApiClient({ service: 'security_alerts_api', logger, }), - logger, }); const snapAssetsAdapter = new SnapAssetsAdapter({