diff --git a/apps/explorer/src/api/operator/operatorApi.ts b/apps/explorer/src/api/operator/operatorApi.ts index e7cb3f141f..9729d34b2e 100644 --- a/apps/explorer/src/api/operator/operatorApi.ts +++ b/apps/explorer/src/api/operator/operatorApi.ts @@ -255,9 +255,15 @@ function ensureSolverCompetition( } function withBarnTimeout(promise: Promise, operation: string): Promise { - return withTimeout(promise, ENV_REQUEST_TIMEOUT_MS, `${operation}: BARN`) + return withTimeout(promise, { + timeout: ENV_REQUEST_TIMEOUT_MS, + timeoutMessage: `${operation}: BARN. Timeout after ${ENV_REQUEST_TIMEOUT_MS} ms`, + }) } function withProdTimeout(promise: Promise, operation: string): Promise { - return withTimeout(promise, ENV_REQUEST_TIMEOUT_MS, `${operation}: PROD`) + return withTimeout(promise, { + timeout: ENV_REQUEST_TIMEOUT_MS, + timeoutMessage: `${operation}: PROD. Timeout after ${ENV_REQUEST_TIMEOUT_MS} ms`, + }) } diff --git a/libs/common-utils/src/index.ts b/libs/common-utils/src/index.ts index cb9c515130..3563926a9e 100644 --- a/libs/common-utils/src/index.ts +++ b/libs/common-utils/src/index.ts @@ -63,7 +63,6 @@ export * from './misc' export * from './node' export * from './normalizeRawAmount' export * from './parseENSAddress' -export * from './promiseWithTimeout' export * from './rawToTokenAmount' export * from './request' export * from './resolveENSContentHash' diff --git a/libs/common-utils/src/misc.test.ts b/libs/common-utils/src/misc.test.ts index 50b2cbf140..78ad2005cc 100644 --- a/libs/common-utils/src/misc.test.ts +++ b/libs/common-utils/src/misc.test.ts @@ -1,4 +1,25 @@ -import { isRejectRequestProviderError } from './misc' +import { isRejectRequestProviderError, TimeoutError, withTimeout } from './misc' + +describe('withTimeout', () => { + it('resolves when the promise settles before the timeout', async () => { + await expect(withTimeout(Promise.resolve('ok'), { timeout: 1000, timeoutMessage: 'Timed out' })).resolves.toBe('ok') + }) + + it('rejects with TimeoutError when the promise times out', async () => { + await expect( + withTimeout(new Promise(() => undefined), { timeout: 50, timeoutMessage: 'Timed out' }), + ).rejects.toThrow(TimeoutError) + }) + + it('rejects with the provided timeout message', async () => { + await expect( + withTimeout(new Promise(() => undefined), { + timeout: 50, + timeoutMessage: 'Timed out', + }), + ).rejects.toThrow('Timed out') + }) +}) describe('isRejectRequestProviderError', () => { it('detects the standard EIP-1193 rejection code', () => { diff --git a/libs/common-utils/src/misc.ts b/libs/common-utils/src/misc.ts index 76e68a949c..0d27f17e32 100644 --- a/libs/common-utils/src/misc.ts +++ b/libs/common-utils/src/misc.ts @@ -60,13 +60,24 @@ export function isPromiseFulfilled( return promiseResult.status === 'fulfilled' } -export function withTimeout(promise: Promise, ms: number, context?: string): Promise { - const failOnTimeout = delay(ms).then(() => { - const errorMessage = 'Timeout after ' + ms + ' ms' - throw new Error(context ? `${context}. ${errorMessage}` : errorMessage) +export class TimeoutError extends Error {} + +interface TimeoutOptions { + timeout: number + timeoutMessage: string +} + +export async function withTimeout(promise: Promise, options: TimeoutOptions): Promise { + const { timeout, timeoutMessage } = options + let timeoutId: ReturnType | undefined + + const failOnTimeout = new Promise((_, reject) => { + timeoutId = setTimeout(() => reject(new TimeoutError(timeoutMessage)), timeout) }) - return Promise.race([promise, failOnTimeout]) + return Promise.race([promise, failOnTimeout]).finally(() => { + clearTimeout(timeoutId) + }) } export const registerOnWindow = (registerMapping: Record): void => { diff --git a/libs/common-utils/src/promiseWithTimeout.test.ts b/libs/common-utils/src/promiseWithTimeout.test.ts deleted file mode 100644 index 450867bc17..0000000000 --- a/libs/common-utils/src/promiseWithTimeout.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { PromiseWithTimeout } from './promiseWithTimeout' - -describe('PromiseWithTimeout', () => { - it('resolves when the handler calls resolve before the timeout', async () => { - await expect(PromiseWithTimeout(1000, (resolve) => resolve('ok'))).resolves.toBe('ok') - }) - - it('rejects when the promise times out before resolve', async () => { - const p = PromiseWithTimeout(50, () => { - /* never resolves */ - }) - await expect(p).rejects.toThrow('Promise timed out after 50ms') - }, 10_000) - - it('clears the timeout when resolve is called', async () => { - await expect( - PromiseWithTimeout(10_000, (resolve) => { - resolve(42) - }), - ).resolves.toBe(42) - }) - - it('rejects when the handler throws synchronously', async () => { - const err = new Error('boom') - const p = PromiseWithTimeout(1000, () => { - throw err - }) - await expect(p).rejects.toBe(err) - }) - - it('works as a constructor: new PromiseWithTimeout(...)', async () => { - const p = new PromiseWithTimeout(1000, (resolve) => resolve('ctor')) - await expect(p).resolves.toBe('ctor') - }) -}) diff --git a/libs/common-utils/src/promiseWithTimeout.ts b/libs/common-utils/src/promiseWithTimeout.ts deleted file mode 100644 index df1cee6c86..0000000000 --- a/libs/common-utils/src/promiseWithTimeout.ts +++ /dev/null @@ -1,44 +0,0 @@ -type PromiseWithTimeoutHandler = ( - resolve: (value: T | PromiseLike) => void, - reject: (reason?: unknown) => void, -) => void - -type PromiseWithTimeoutType = { - (timeoutMs: number, handler: PromiseWithTimeoutHandler): Promise - new (timeoutMs: number, handler: PromiseWithTimeoutHandler): Promise -} - -/** - * Returns a Promise that runs the given handler with a timeout. If the handler - * does not call resolve/reject before the timeout, the promise rejects. - * - * Usage: await new PromiseWithTimeout(ms, (resolve, reject) => { ... }) - * or: await PromiseWithTimeout(ms, (resolve, reject) => { ... }) - */ -const promiseWithTimeoutImpl = function PromiseWithTimeout( - timeoutMs: number, - handler: PromiseWithTimeoutHandler, -): Promise { - return new Promise((resolve, reject) => { - const timeoutId = setTimeout(() => { - reject(new Error(`Promise timed out after ${timeoutMs}ms`)) - }, timeoutMs) - - const wrapResolve = (value: T | PromiseLike): void => { - clearTimeout(timeoutId) - resolve(value) - } - const wrapReject = (reason?: unknown): void => { - clearTimeout(timeoutId) - reject(reason) - } - - try { - handler(wrapResolve, wrapReject) - } catch (e) { - wrapReject(e) - } - }) -} - -export const PromiseWithTimeout = promiseWithTimeoutImpl as PromiseWithTimeoutType diff --git a/libs/wallet/src/api/state/walletCapabilitiesAtom.test.ts b/libs/wallet/src/api/state/walletCapabilitiesAtom.test.ts index 48250d1c7e..7a08371c85 100644 --- a/libs/wallet/src/api/state/walletCapabilitiesAtom.test.ts +++ b/libs/wallet/src/api/state/walletCapabilitiesAtom.test.ts @@ -1,6 +1,6 @@ import { createStore, type WritableAtom } from 'jotai' -import type { EIP1193Provider } from 'viem' +import type { EIP1193Provider, PublicClient } from 'viem' import type { Connector } from 'wagmi' import { SupportedChainId } from '@cowprotocol/cow-sdk' @@ -54,25 +54,22 @@ const MOCK_ACCOUNT = '0x1234567890123456789012345678901234567890' as const const MOCK_CHAIN_ID = SupportedChainId.MAINNET const MOCK_CONNECTOR = { type: 'injected' } as Connector -const mockLogWalletWarn = jest.fn() const mockGetCapabilities = jest.fn() const mockWagmiConfigGetClient = jest.fn() const mockGetIsWalletConnect = jest.fn() const mockIsMobile = { value: false } -jest.mock('@cowprotocol/common-utils', () => ({ - getCurrentChainIdFromUrl: () => 1, - get isMobile() { - return mockIsMobile.value - }, - logWallet: { - warn: (...args: unknown[]) => mockLogWalletWarn(...args), - }, - PromiseWithTimeout: (_: number, handler: (resolve: (value: T) => void) => void): Promise => - new Promise((resolve) => { - handler(resolve) - }), -})) +jest.mock('@cowprotocol/common-utils', () => { + const actual = jest.requireActual('@cowprotocol/common-utils') + + return { + ...actual, + getCurrentChainIdFromUrl: () => 1, + get isMobile() { + return mockIsMobile.value + }, + } +}) jest.mock('../../wagmi/hooks/useIsWalletConnect', () => ({ getIsWalletConnect: (...args: unknown[]) => mockGetIsWalletConnect(...args), @@ -122,7 +119,7 @@ function setWalletInfo( account: string chainId: SupportedChainId connector: Connector - provider: EIP1193Provider + provider: NonNullable }> = {}, ): void { store.set(walletInfoAtom, { @@ -304,21 +301,34 @@ describe('walletCapabilitiesAtom', () => { }) }) - it('returns empty object when getCapabilities does not settle before timeout', async () => { + it('returns null when getCapabilities fails and provider does not support EIP-1193 requests', async () => { + const error = new Error('viem error') + mockGetCapabilities.mockRejectedValue(error) + + const store = createStore() + setWalletInfo(store, { provider: {} as PublicClient }) + + const result = await store.get(walletCapabilitiesAtom) + + expect(result).toBeNull() + }) + + it('returns null when getCapabilities times out and provider does not support EIP-1193 requests', async () => { jest.useFakeTimers() mockGetCapabilities.mockImplementation(() => new Promise(() => undefined)) const store = createStore() - setWalletInfo(store) - - const resultPromise = store.get(walletCapabilitiesAtom) - await jest.advanceTimersByTimeAsync(5_000) - const result = await resultPromise + setWalletInfo(store, { provider: {} as PublicClient }) - expect(result).toEqual({}) - expect(mockLogWalletWarn).toHaveBeenCalledWith(expect.stringContaining('Wallet capabilities loading timed out')) + try { + const resultPromise = store.get(walletCapabilitiesAtom) + await jest.advanceTimersByTimeAsync(5_000) + const result = await resultPromise - jest.useRealTimers() + expect(result).toBeNull() + } finally { + jest.useRealTimers() + } }) }) @@ -348,6 +358,49 @@ describe('walletCapabilitiesAtom', () => { expect(result).toEqual(capabilities) }) + + it('uses legacy fallback when getCapabilities times out', async () => { + jest.useFakeTimers() + const capabilities: WalletCapabilities = { atomic: { status: 'supported' } } + mockGetCapabilities.mockImplementation(() => new Promise(() => undefined)) + const mockRequest = jest.fn().mockResolvedValue({ '0x1': capabilities }) + + const store = createStore() + setWalletInfo(store, { provider: createMockEip1193Provider(mockRequest) }) + + try { + const resultPromise = store.get(walletCapabilitiesAtom) + await jest.advanceTimersByTimeAsync(5_000) + const result = await resultPromise + + expect(result).toEqual(capabilities) + expect(mockRequest).toHaveBeenCalledWith({ + method: 'wallet_getCapabilities', + params: [MOCK_ACCOUNT], + }) + } finally { + jest.useRealTimers() + } + }) + + it('returns null when legacy fallback times out', async () => { + jest.useFakeTimers() + mockGetCapabilities.mockRejectedValue(new Error('viem error')) + const mockRequest = jest.fn().mockImplementation(() => new Promise(() => undefined)) + + const store = createStore() + setWalletInfo(store, { provider: createMockEip1193Provider(mockRequest) }) + + try { + const resultPromise = store.get(walletCapabilitiesAtom) + await jest.advanceTimersByTimeAsync(5_000) + const result = await resultPromise + + expect(result).toBeNull() + } finally { + jest.useRealTimers() + } + }) }) }) diff --git a/libs/wallet/src/api/state/walletCapabilitiesAtom.ts b/libs/wallet/src/api/state/walletCapabilitiesAtom.ts index e2ff15573f..24d34f3e7f 100644 --- a/libs/wallet/src/api/state/walletCapabilitiesAtom.ts +++ b/libs/wallet/src/api/state/walletCapabilitiesAtom.ts @@ -4,7 +4,7 @@ import { loadable } from 'jotai/utils' import { EIP1193Provider, numberToHex, PublicClient } from 'viem' import { Connector } from 'wagmi' -import { isMobile, logWallet, PromiseWithTimeout } from '@cowprotocol/common-utils' +import { isMobile, logWallet, normalizeError, TimeoutError, withTimeout } from '@cowprotocol/common-utils' import { ProviderMetaInfoPayload, WidgetEthereumProvider } from '@cowprotocol/iframe-transport' import { AccountType } from '@cowprotocol/types' @@ -25,9 +25,7 @@ import { walletInfoAtom } from '../state' export type WalletCapabilities = GetCapabilitiesReturnType[number] -const WALLET_CAPABILITIES_LOADING_TIMEOUT = ms`5s` - -let timeoutLogged = false +const REQUEST_TIMEOUT_MS = ms`5s` /** * WalletConnect in mobile browsers initiates a request with confirmation to the wallet @@ -49,18 +47,6 @@ export async function getShouldSkipCapabilitiesCheck( return isWalletConnectViaWidget } -function getTimeoutPromise(): Promise { - return new Promise((resolve) => setTimeout(() => resolve(), WALLET_CAPABILITIES_LOADING_TIMEOUT)).then(() => { - if (!timeoutLogged) { - timeoutLogged = true - logWallet.warn(`Wallet capabilities loading timed out after ${WALLET_CAPABILITIES_LOADING_TIMEOUT / 1000}s`) - } - return {} as T - }) -} - -const REQUEST_TIMEOUT_MS = ms`5s` - /** * Safe WC returns EIP-5792 capabilities keyed by hex chain id (e.g. "0xaa36a7") * while walletInfoAtom.chainId is numeric (e.g. 11155111). Numeric lookup alone misses them. @@ -84,11 +70,16 @@ async function fetchWidgetProviderMetaInfo( provider: EIP1193Provider | WidgetEthereumProvider | PublicClient, ): Promise { if (provider instanceof WidgetEthereumProvider) { - return PromiseWithTimeout(REQUEST_TIMEOUT_MS, (resolve) => { + const providerMetaInfo = new Promise((resolve) => { provider.onProviderMetaInfo((data) => { provider.clearProviderMetaInfoListener() resolve(data) }) + }) + + return withTimeout(providerMetaInfo, { + timeout: REQUEST_TIMEOUT_MS, + timeoutMessage: `Widget provider meta info request timed out after ${REQUEST_TIMEOUT_MS}ms`, }).catch(() => { provider.clearProviderMetaInfoListener() return null @@ -103,6 +94,7 @@ async function fetchWidgetProviderMetaInfo( * Returns capabilities for the current account and chain, or null when disconnected or on error. */ +// eslint-disable-next-line complexity export const walletCapabilitiesAtom = atom(async (get): Promise => { const { account, chainId, provider, connector } = get(walletInfoAtom) const isSafeViaWc = get(isSafeViaWcAtom) @@ -168,38 +160,53 @@ export const walletCapabilitiesAtom = atom(async (get): Promise()]) - } catch (getCapabilitiesError) { if (!isEip1193Provider(provider)) { - console.error('Cannot fetch wallet capabilities', getCapabilitiesError) - throw getCapabilitiesError + logWallet.error('Cannot fetch wallet capabilities via wagmi', { account, chainId }, wagmiError) + return null } try { - const legacyCapabilitiesPromise = provider.request({ - method: 'wallet_getCapabilities', - params: [account], - }) - - const legacyCapabilities = await Promise.race([ - legacyCapabilitiesPromise, - getTimeoutPromise(), - ]) + const legacyCapabilities = await withTimeout( + provider.request({ + method: 'wallet_getCapabilities', + params: [account], + }), + { + timeout: REQUEST_TIMEOUT_MS, + timeoutMessage: `Wallet capabilities loading timed out after ${REQUEST_TIMEOUT_MS / 1000}s`, + }, + ) capabilities = resolveCapabilitiesForChain(legacyCapabilities, chainId) - console.warn('getCapabilities() failed, but wallet_getCapabilities returned capabilities', legacyCapabilities) - } catch (walletGetCapabilitiesError) { - console.error( - 'Both getCapabilities() and wallet_getCapabilities failed', - getCapabilitiesError, - walletGetCapabilitiesError, - ) + logWallet.warn('getCapabilities() failed, but wallet_getCapabilities returned capabilities', legacyCapabilities) + } catch (err: unknown) { + const rpcError = normalizeError(err) + + if (rpcError instanceof TimeoutError) { + logWallet.warn(rpcError.message) + } else { + logWallet.error('Cannot fetch wallet capabilities via rpc', { account, chainId }, rpcError) + } + + return null } }