diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 91149d41af..c4f51b34f5 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -5,6 +5,10 @@ { "note": "Use SOURCE_FLAGS.rfqOrder in comparisonPrice", "pr": 177 + }, + { + "note": "Add a cancel token to ensure timeouts are respected", + "pr": 176 } ] }, diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 2a83b33b33..7e43545895 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -368,6 +368,7 @@ export interface MockedRfqtQuoteResponse { requestParams: TakerRequestQueryParams; responseData: any; responseCode: number; + callback?: (config: any) => Promise; } /** diff --git a/packages/asset-swapper/src/utils/alt_mm_implementation_utils.ts b/packages/asset-swapper/src/utils/alt_mm_implementation_utils.ts index 42006bbe50..3b151189d1 100644 --- a/packages/asset-swapper/src/utils/alt_mm_implementation_utils.ts +++ b/packages/asset-swapper/src/utils/alt_mm_implementation_utils.ts @@ -1,7 +1,7 @@ import { Web3Wrapper } from '@0x/dev-utils'; import { TakerRequestQueryParams, V4RFQFirmQuote, V4RFQIndicativeQuote } from '@0x/quote-server'; import { BigNumber } from '@0x/utils'; -import { AxiosInstance } from 'axios'; +import { AxiosInstance, CancelToken } from 'axios'; import { constants } from '../constants'; import { @@ -122,6 +122,7 @@ export async function returnQuoteFromAltMMAsync( altRfqtAssetOfferings: AltRfqtMakerAssetOfferings, takerRequestQueryParams: TakerRequestQueryParams, axiosInstance: AxiosInstance, + cancelToken: CancelToken, ): Promise<{ data: ResponseT; status: number }> { const altPair = getAltMarketInfo( altRfqtAssetOfferings[url], @@ -214,6 +215,7 @@ export async function returnQuoteFromAltMMAsync( const response = await axiosInstance.post(`${url}/quotes`, data, { headers: { Authorization: `Bearer ${apiKey}` }, timeout: maxResponseTimeMs, + cancelToken, }); if (response.data.status === 'rejected') { diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index cc00a095a5..a96b7c2fcd 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -2,7 +2,7 @@ import { schemas, SchemaValidator } from '@0x/json-schemas'; import { FillQuoteTransformerOrderType, Signature } from '@0x/protocol-utils'; import { TakerRequestQueryParams, V4RFQFirmQuote, V4RFQIndicativeQuote, V4SignedRfqOrder } from '@0x/quote-server'; import { BigNumber, NULL_ADDRESS } from '@0x/utils'; -import { AxiosInstance } from 'axios'; +import axios, { AxiosInstance } from 'axios'; import { constants } from '../constants'; import { @@ -392,6 +392,17 @@ export class QuoteRequestor { const typedMakerUrls = standardUrls.concat(altUrls); + const timeoutMs = + options.makerEndpointMaxResponseTimeMs || + constants.DEFAULT_RFQT_REQUEST_OPTS.makerEndpointMaxResponseTimeMs!; + const bufferMs = 20; + + // Set Timeout on CancelToken + const cancelTokenSource = axios.CancelToken.source(); + setTimeout(() => { + cancelTokenSource.cancel('timeout via cancel token'); + }, timeoutMs + bufferMs); + const quotePromises = typedMakerUrls.map(async typedMakerUrl => { // filter out requests to skip const isBlacklisted = rfqMakerBlacklist.isMakerBlacklisted(typedMakerUrl.url); @@ -404,16 +415,13 @@ export class QuoteRequestor { } else { // make request to MM const timeBeforeAwait = Date.now(); - const maxResponseTimeMs = - options.makerEndpointMaxResponseTimeMs === undefined - ? constants.DEFAULT_RFQT_REQUEST_OPTS.makerEndpointMaxResponseTimeMs! - : options.makerEndpointMaxResponseTimeMs; try { if (typedMakerUrl.pairType === RfqPairType.Standard) { const response = await this._quoteRequestorHttpClient.get(`${typedMakerUrl.url}/${quotePath}`, { headers: { '0x-api-key': options.apiKey }, params: requestParams, - timeout: maxResponseTimeMs, + timeout: timeoutMs, + cancelToken: cancelTokenSource.token, }); const latencyMs = Date.now() - timeBeforeAwait; this._infoLogger({ @@ -429,7 +437,7 @@ export class QuoteRequestor { }, }, }); - rfqMakerBlacklist.logTimeoutOrLackThereof(typedMakerUrl.url, latencyMs >= maxResponseTimeMs); + rfqMakerBlacklist.logTimeoutOrLackThereof(typedMakerUrl.url, latencyMs >= timeoutMs); return { response: response.data, makerUri: typedMakerUrl.url }; } else { if (this._altRfqCreds === undefined) { @@ -443,10 +451,11 @@ export class QuoteRequestor { quoteType === 'firm' ? AltQuoteModel.Firm : AltQuoteModel.Indicative, makerToken, takerToken, - maxResponseTimeMs, + timeoutMs, options.altRfqtAssetOfferings || {}, requestParams, this._quoteRequestorHttpClient, + cancelTokenSource.token, ); const latencyMs = Date.now() - timeBeforeAwait; @@ -463,7 +472,7 @@ export class QuoteRequestor { }, }, }); - rfqMakerBlacklist.logTimeoutOrLackThereof(typedMakerUrl.url, latencyMs >= maxResponseTimeMs); + rfqMakerBlacklist.logTimeoutOrLackThereof(typedMakerUrl.url, latencyMs >= timeoutMs); return { response: quote.data, makerUri: typedMakerUrl.url }; } } catch (err) { @@ -482,7 +491,7 @@ export class QuoteRequestor { }, }, }); - rfqMakerBlacklist.logTimeoutOrLackThereof(typedMakerUrl.url, latencyMs >= maxResponseTimeMs); + rfqMakerBlacklist.logTimeoutOrLackThereof(typedMakerUrl.url, latencyMs >= timeoutMs); this._warningLogger( convertIfAxiosError(err), `Failed to get RFQ-T ${quoteType} quote from market maker endpoint ${ @@ -495,6 +504,7 @@ export class QuoteRequestor { } } }); + const results = (await Promise.all(quotePromises)).filter(x => x !== undefined); return results as Array>; } diff --git a/packages/asset-swapper/test/quote_requestor_test.ts b/packages/asset-swapper/test/quote_requestor_test.ts index 9e6246c187..a1ae63de82 100644 --- a/packages/asset-swapper/test/quote_requestor_test.ts +++ b/packages/asset-swapper/test/quote_requestor_test.ts @@ -3,7 +3,7 @@ import { FillQuoteTransformerOrderType, SignatureType } from '@0x/protocol-utils import { TakerRequestQueryParams, V4RFQIndicativeQuote } from '@0x/quote-server'; import { StatusCodes } from '@0x/types'; import { BigNumber, logUtils } from '@0x/utils'; -import Axios, { AxiosInstance } from 'axios'; +import Axios from 'axios'; import * as chai from 'chai'; import { Agent as HttpAgent } from 'http'; import { Agent as HttpsAgent } from 'https'; @@ -366,6 +366,93 @@ describe('QuoteRequestor', async () => { quoteRequestorHttpClient, ); }); + it('should only return RFQT requests that meet the timeout', async () => { + const takerAddress = '0xd209925defc99488e3afff1174e48b4fa628302a'; + const apiKey = 'my-ko0l-api-key'; + const maxTimeoutMs = 10; + // tslint:disable-next-line:custom-no-magic-numbers + const exceedTimeoutMs = maxTimeoutMs + 50; + + // Set up RFQT responses + // tslint:disable-next-line:array-type + const mockedRequests: MockedRfqtQuoteResponse[] = []; + const expectedParams: TakerRequestQueryParams = { + sellTokenAddress: takerToken, + buyTokenAddress: makerToken, + sellAmountBaseUnits: '10000', + comparisonPrice: undefined, + takerAddress, + txOrigin: takerAddress, + protocolVersion: '4', + }; + const mockedDefaults = { + requestApiKey: apiKey, + requestParams: expectedParams, + responseCode: StatusCodes.Success, + }; + + // Successful response + const successfulQuote1 = { + makerToken, + takerToken, + makerAmount: new BigNumber(expectedParams.sellAmountBaseUnits), + takerAmount: new BigNumber(expectedParams.sellAmountBaseUnits), + expiry: makeThreeMinuteExpiry(), + }; + + // One good request + mockedRequests.push({ + ...mockedDefaults, + endpoint: 'https://1337.0.0.1', + responseData: successfulQuote1, + }); + + // One request that will timeout + mockedRequests.push({ + ...mockedDefaults, + endpoint: 'https://420.0.0.1', + responseData: successfulQuote1, + callback: async () => { + // tslint:disable-next-line:no-inferred-empty-object-type + return new Promise((resolve, reject) => { + setTimeout(() => { + resolve([StatusCodes.Success, successfulQuote1]); + }, exceedTimeoutMs); + }); + }, + }); + + return testHelpers.withMockedRfqtQuotes( + mockedRequests, + [], + RfqtQuoteEndpoint.Indicative, + async () => { + const qr = new QuoteRequestor( + { + 'https://1337.0.0.1': [[makerToken, takerToken]], + 'https://420.0.0.1': [[makerToken, takerToken]], + }, + quoteRequestorHttpClient, + ); + const resp = await qr.requestRfqtIndicativeQuotesAsync( + makerToken, + takerToken, + new BigNumber(10000), + MarketOperation.Sell, + undefined, + { + apiKey, + takerAddress, + txOrigin: takerAddress, + intentOnFilling: true, + makerEndpointMaxResponseTimeMs: maxTimeoutMs, + }, + ); + expect(resp.sort()).to.eql([successfulQuote1].sort()); // notice only one result, despite two requests made + }, + quoteRequestorHttpClient, + ); + }); it('should return successful RFQT indicative quote requests (Buy)', async () => { const takerAddress = '0xd209925defc99488e3afff1174e48b4fa628302a'; const apiKey = 'my-ko0l-api-key'; diff --git a/packages/asset-swapper/test/utils/test_helpers.ts b/packages/asset-swapper/test/utils/test_helpers.ts index 0caf4d3119..297705cca9 100644 --- a/packages/asset-swapper/test/utils/test_helpers.ts +++ b/packages/asset-swapper/test/utils/test_helpers.ts @@ -49,9 +49,15 @@ export const testHelpers = { for (const mockedResponse of standardMockedResponses) { const { endpoint, requestApiKey, requestParams, responseData, responseCode } = mockedResponse; const requestHeaders = { Accept: 'application/json, text/plain, */*', '0x-api-key': requestApiKey }; - mockedAxios - .onGet(`${endpoint}/${quoteType}`, { params: requestParams }, requestHeaders) - .replyOnce(responseCode, responseData); + if (mockedResponse.callback !== undefined) { + mockedAxios + .onGet(`${endpoint}/${quoteType}`, { params: requestParams }, requestHeaders) + .reply(mockedResponse.callback); + } else { + mockedAxios + .onGet(`${endpoint}/${quoteType}`, { params: requestParams }, requestHeaders) + .replyOnce(responseCode, responseData); + } } // Mock out Alt RFQT responses for (const mockedResponse of altMockedResponses) {