From 7159dd68679b427cb62ad96eb984a7b6fa5b642f Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 19 Jan 2021 08:41:57 -0800 Subject: [PATCH] Fetch with a timeout everywhere (#10101) * Use fetchWithTimeout everywhere * Memoize getFetchWithTimeout * Require specified timeout --- .../controllers/incoming-transactions.js | 8 ++--- app/scripts/controllers/token-rates.js | 5 +++- app/scripts/lib/ens-ipfs/setup.js | 7 ++++- app/scripts/lib/network-store.js | 5 +++- .../modules}/fetch-with-timeout.js | 12 ++++++-- test/unit/app/fetch-with-timeout.test.js | 29 +++++++++++-------- ui/app/ducks/gas/gas.duck.js | 6 ++-- ui/app/helpers/utils/fetch-with-cache.js | 6 ++-- ui/app/helpers/utils/i18n-helper.js | 9 ++++-- ui/app/helpers/utils/util.js | 27 ++++++++--------- 10 files changed, 70 insertions(+), 44 deletions(-) rename {app/scripts/lib => shared/modules}/fetch-with-timeout.js (64%) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 58a059dd2..625272740 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/app/scripts/controllers/incoming-transactions.js @@ -3,7 +3,7 @@ import log from 'loglevel' import BN from 'bn.js' import createId from '../lib/random-id' import { bnToHex } from '../lib/util' -import fetchWithTimeout from '../lib/fetch-with-timeout' +import getFetchWithTimeout from '../../../shared/modules/fetch-with-timeout' import { TRANSACTION_CATEGORIES, @@ -24,9 +24,7 @@ import { ROPSTEN_CHAIN_ID, } from './network/enums' -const fetch = fetchWithTimeout({ - timeout: 30000, -}) +const fetchWithTimeout = getFetchWithTimeout(30000) /** * This controller is responsible for retrieving incoming transactions. Etherscan is polled once every block to check @@ -227,7 +225,7 @@ export default class IncomingTransactionsController { if (fromBlock) { url += `&startBlock=${parseInt(fromBlock, 10)}` } - const response = await fetch(url) + const response = await fetchWithTimeout(url) const parsedResponse = await response.json() return { diff --git a/app/scripts/controllers/token-rates.js b/app/scripts/controllers/token-rates.js index 9011b1d01..031017c83 100644 --- a/app/scripts/controllers/token-rates.js +++ b/app/scripts/controllers/token-rates.js @@ -2,6 +2,9 @@ import { ObservableStore } from '@metamask/obs-store' import log from 'loglevel' import { normalize as normalizeAddress } from 'eth-sig-util' import ethUtil from 'ethereumjs-util' +import getFetchWithTimeout from '../../../shared/modules/fetch-with-timeout' + +const fetchWithTimeout = getFetchWithTimeout(30000) // By default, poll every 3 minutes const DEFAULT_INTERVAL = 180 * 1000 @@ -34,7 +37,7 @@ export default class TokenRatesController { const query = `contract_addresses=${pairs}&vs_currencies=${nativeCurrency}` if (this._tokens.length > 0) { try { - const response = await window.fetch( + const response = await fetchWithTimeout( `https://api.coingecko.com/api/v3/simple/token_price/ethereum?${query}`, ) const prices = await response.json() diff --git a/app/scripts/lib/ens-ipfs/setup.js b/app/scripts/lib/ens-ipfs/setup.js index ab4cec023..9ec6e1492 100644 --- a/app/scripts/lib/ens-ipfs/setup.js +++ b/app/scripts/lib/ens-ipfs/setup.js @@ -1,6 +1,9 @@ import extension from 'extensionizer' +import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout' import resolveEnsToIpfsContentId from './resolver' +const fetchWithTimeout = getFetchWithTimeout(30000) + const supportedTopLevelDomains = ['eth'] export default function setupEnsIpfsResolver({ @@ -55,7 +58,9 @@ export default function setupEnsIpfsResolver({ )}.${ipfsGateway}${pathname}${search || ''}${fragment || ''}` try { // check if ipfs gateway has result - const response = await window.fetch(resolvedUrl, { method: 'HEAD' }) + const response = await fetchWithTimeout(resolvedUrl, { + method: 'HEAD', + }) if (response.status === 200) { url = resolvedUrl } diff --git a/app/scripts/lib/network-store.js b/app/scripts/lib/network-store.js index cc2898292..bfa7a8ba4 100644 --- a/app/scripts/lib/network-store.js +++ b/app/scripts/lib/network-store.js @@ -1,4 +1,7 @@ import log from 'loglevel' +import getFetchWithTimeout from '../../../shared/modules/fetch-with-timeout' + +const fetchWithTimeout = getFetchWithTimeout(30000) const FIXTURE_SERVER_HOST = 'localhost' const FIXTURE_SERVER_PORT = 12345 @@ -24,7 +27,7 @@ export default class ReadOnlyNetworkStore { */ async _init() { try { - const response = await window.fetch(FIXTURE_SERVER_URL) + const response = await fetchWithTimeout(FIXTURE_SERVER_URL) if (response.ok) { this._state = await response.json() } diff --git a/app/scripts/lib/fetch-with-timeout.js b/shared/modules/fetch-with-timeout.js similarity index 64% rename from app/scripts/lib/fetch-with-timeout.js rename to shared/modules/fetch-with-timeout.js index a9e895b12..e97ae3402 100644 --- a/app/scripts/lib/fetch-with-timeout.js +++ b/shared/modules/fetch-with-timeout.js @@ -1,4 +1,10 @@ -const fetchWithTimeout = ({ timeout = 120000 } = {}) => { +import { memoize } from 'lodash' + +const getFetchWithTimeout = memoize((timeout) => { + if (!Number.isInteger(timeout) || timeout < 1) { + throw new Error('Must specify positive integer timeout.') + } + return async function _fetch(url, opts) { const abortController = new window.AbortController() const { signal } = abortController @@ -18,6 +24,6 @@ const fetchWithTimeout = ({ timeout = 120000 } = {}) => { throw e } } -} +}) -export default fetchWithTimeout +export default getFetchWithTimeout diff --git a/test/unit/app/fetch-with-timeout.test.js b/test/unit/app/fetch-with-timeout.test.js index 89ea2fec9..dd91ff58b 100644 --- a/test/unit/app/fetch-with-timeout.test.js +++ b/test/unit/app/fetch-with-timeout.test.js @@ -1,14 +1,16 @@ import assert from 'assert' import nock from 'nock' -import fetchWithTimeout from '../../../app/scripts/lib/fetch-with-timeout' +import getFetchWithTimeout from '../../../shared/modules/fetch-with-timeout' -describe('fetchWithTimeout', function () { +describe('getFetchWithTimeout', function () { it('fetches a url', async function () { nock('https://api.infura.io').get('/money').reply(200, '{"hodl": false}') - const fetch = fetchWithTimeout() - const response = await (await fetch('https://api.infura.io/money')).json() + const fetchWithTimeout = getFetchWithTimeout(30000) + const response = await ( + await fetchWithTimeout('https://api.infura.io/money') + ).json() assert.deepEqual(response, { hodl: false, }) @@ -20,12 +22,10 @@ describe('fetchWithTimeout', function () { .delay(2000) .reply(200, '{"moon": "2012-12-21T11:11:11Z"}') - const fetch = fetchWithTimeout({ - timeout: 123, - }) + const fetchWithTimeout = getFetchWithTimeout(123) try { - await fetch('https://api.infura.io/moon').then((r) => r.json()) + await fetchWithTimeout('https://api.infura.io/moon').then((r) => r.json()) assert.fail('Request should throw') } catch (e) { assert.ok(e) @@ -38,15 +38,20 @@ describe('fetchWithTimeout', function () { .delay(2000) .reply(200, '{"moon": "2012-12-21T11:11:11Z"}') - const fetch = fetchWithTimeout({ - timeout: 123, - }) + const fetchWithTimeout = getFetchWithTimeout(123) try { - await fetch('https://api.infura.io/moon').then((r) => r.json()) + await fetchWithTimeout('https://api.infura.io/moon').then((r) => r.json()) assert.fail('Request should be aborted') } catch (e) { assert.deepEqual(e.message, 'Aborted') } }) + + it('throws on invalid timeout', async function () { + assert.throws(() => getFetchWithTimeout(), 'should throw') + assert.throws(() => getFetchWithTimeout(-1), 'should throw') + assert.throws(() => getFetchWithTimeout({}), 'should throw') + assert.throws(() => getFetchWithTimeout(true), 'should throw') + }) }) diff --git a/ui/app/ducks/gas/gas.duck.js b/ui/app/ducks/gas/gas.duck.js index 83b15ecf1..1bcc5d417 100644 --- a/ui/app/ducks/gas/gas.duck.js +++ b/ui/app/ducks/gas/gas.duck.js @@ -1,8 +1,10 @@ import { cloneDeep } from 'lodash' import BigNumber from 'bignumber.js' import { getStorageItem, setStorageItem } from '../../../lib/storage-helpers' - import { decGWEIToHexWEI } from '../../helpers/utils/conversions.util' +import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout' + +const fetchWithTimeout = getFetchWithTimeout(30000) // Actions const BASIC_GAS_ESTIMATE_LOADING_FINISHED = @@ -97,7 +99,7 @@ export function basicGasEstimatesLoadingFinished() { async function basicGasPriceQuery() { const url = `https://api.metaswap.codefi.network/gasPrices` - return await window.fetch(url, { + return await fetchWithTimeout(url, { headers: {}, referrer: 'https://api.metaswap.codefi.network/gasPrices', referrerPolicy: 'no-referrer-when-downgrade', diff --git a/ui/app/helpers/utils/fetch-with-cache.js b/ui/app/helpers/utils/fetch-with-cache.js index ca2bbd7f0..a903680c4 100644 --- a/ui/app/helpers/utils/fetch-with-cache.js +++ b/ui/app/helpers/utils/fetch-with-cache.js @@ -1,5 +1,5 @@ import { getStorageItem, setStorageItem } from '../../../lib/storage-helpers' -import fetchWithTimeout from '../../../../app/scripts/lib/fetch-with-timeout' +import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout' const fetchWithCache = async ( url, @@ -29,8 +29,8 @@ const fetchWithCache = async ( return cachedResponse } fetchOptions.headers.set('Content-Type', 'application/json') - const _fetch = timeout ? fetchWithTimeout({ timeout }) : window.fetch - const response = await _fetch(url, { + const fetchWithTimeout = getFetchWithTimeout(timeout) + const response = await fetchWithTimeout(url, { referrerPolicy: 'no-referrer-when-downgrade', body: null, method: 'GET', diff --git a/ui/app/helpers/utils/i18n-helper.js b/ui/app/helpers/utils/i18n-helper.js index 140a12fff..bb8932683 100644 --- a/ui/app/helpers/utils/i18n-helper.js +++ b/ui/app/helpers/utils/i18n-helper.js @@ -1,9 +1,12 @@ // cross-browser connection to extension i18n API import React from 'react' import log from 'loglevel' - import * as Sentry from '@sentry/browser' +import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout' + +const fetchWithTimeout = getFetchWithTimeout(30000) + const warned = {} const missingMessageErrors = {} const missingSubstitutionErrors = {} @@ -95,7 +98,7 @@ export const getMessage = (localeCode, localeMessages, key, substitutions) => { export async function fetchLocale(localeCode) { try { - const response = await window.fetch( + const response = await fetchWithTimeout( `./_locales/${localeCode}/messages.json`, ) return await response.json() @@ -120,7 +123,7 @@ export async function loadRelativeTimeFormatLocaleData(localeCode) { } async function fetchRelativeTimeFormatData(languageTag) { - const response = await window.fetch( + const response = await fetchWithTimeout( `./intl/${languageTag}/relative-time-format-data.json`, ) return await response.json() diff --git a/ui/app/helpers/utils/util.js b/ui/app/helpers/utils/util.js index 020ed9325..b88abc876 100644 --- a/ui/app/helpers/utils/util.js +++ b/ui/app/helpers/utils/util.js @@ -4,6 +4,9 @@ import BigNumber from 'bignumber.js' import ethUtil from 'ethereumjs-util' import { DateTime } from 'luxon' import { addHexPrefix } from '../../../../app/scripts/lib/util' +import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout' + +const fetchWithTimeout = getFetchWithTimeout(30000) // formatData :: ( date: ) -> String export function formatDate(date, format = "M/d/y 'at' T") { @@ -478,19 +481,17 @@ export async function jsonRpcRequest(rpcUrl, rpcMethod, rpcParams = []) { headers.Authorization = `Basic ${encodedAuth}` fetchUrl = `${origin}${pathname}${search}` } - const jsonRpcResponse = await window - .fetch(fetchUrl, { - method: 'POST', - body: JSON.stringify({ - id: Date.now().toString(), - jsonrpc: '2.0', - method: rpcMethod, - params: rpcParams, - }), - headers, - cache: 'default', - }) - .then((httpResponse) => httpResponse.json()) + const jsonRpcResponse = await fetchWithTimeout(fetchUrl, { + method: 'POST', + body: JSON.stringify({ + id: Date.now().toString(), + jsonrpc: '2.0', + method: rpcMethod, + params: rpcParams, + }), + headers, + cache: 'default', + }).then((httpResponse) => httpResponse.json()) if ( !jsonRpcResponse ||