From 24397c51a8c7bf704948c8fc6874843bccd5d244 Mon Sep 17 00:00:00 2001 From: Alex Kroeger Date: Wed, 24 Mar 2021 13:45:54 -0700 Subject: [PATCH] improve logging for alt rfq request (#158) * improve logging for alt rfq request * clean up unsuccessful status code logic * Fix quote requestor tests * get rid of unnecessary promise handling * remove unused code * update changelog * changed warning message for no quote * appease prettier --- packages/asset-swapper/CHANGELOG.json | 4 ++ .../src/utils/alt_mm_implementation_utils.ts | 46 ++++++++++++++++--- .../src/utils/quote_requestor.ts | 3 +- .../test/quote_requestor_test.ts | 12 +++-- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 5b684c6a8d..a7c3fc832b 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -13,6 +13,10 @@ { "note": "Rename {Rfqt=>Rfq} for many types in Asset Swapper", "pr": 179 + }, + { + "note": "improve logging for alt RFQ requests", + "pr": 158 } ] }, 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 e2da1ea1d7..b5806cf659 100644 --- a/packages/asset-swapper/src/utils/alt_mm_implementation_utils.ts +++ b/packages/asset-swapper/src/utils/alt_mm_implementation_utils.ts @@ -12,8 +12,11 @@ import { AltQuoteRequestData, AltQuoteSide, AltRfqMakerAssetOfferings, + LogFunction, } from '../types'; +const SUCCESS_CODE = 201; + function getAltMarketInfo( offerings: AltOffering[], buyTokenAddress: string, @@ -122,6 +125,7 @@ export async function returnQuoteFromAltMMAsync( altRfqAssetOfferings: AltRfqMakerAssetOfferings, takerRequestQueryParams: TakerRequestQueryParams, axiosInstance: AxiosInstance, + warningLogger: LogFunction, cancelToken: CancelToken, ): Promise<{ data: ResponseT; status: number }> { const altPair = getAltMarketInfo( @@ -212,14 +216,44 @@ export async function returnQuoteFromAltMMAsync( } } - const response = await axiosInstance.post(`${url}/quotes`, data, { - headers: { Authorization: `Bearer ${apiKey}` }, - timeout: maxResponseTimeMs, - cancelToken, - }); + const response = await axiosInstance + .post(`${url}/quotes`, data, { + headers: { Authorization: `Bearer ${apiKey}` }, + timeout: maxResponseTimeMs, + cancelToken, + }) + .catch(err => { + warningLogger(err, `Alt RFQ MM request failed`); + throw new Error(`Alt RFQ MM request failed`); + }); + // empty response will get filtered out in validation + const emptyResponse = {}; + + // tslint:disable-next-line:custom-no-magic-numbers + if (response.status !== SUCCESS_CODE) { + const rejectedRequestInfo = { + status: response.status, + message: response.data, + }; + warningLogger(rejectedRequestInfo, `Alt RFQ MM did not return a status of ${SUCCESS_CODE}`); + return { + data: (emptyResponse as unknown) as ResponseT, + status: response.status, + }; + } + // successful handling but no quote is indicated by status = 'rejected' if (response.data.status === 'rejected') { - throw new Error('alt MM rejected quote'); + warningLogger( + response.data.id, + `Alt RFQ MM handled the request successfully but did not return a quote (status = 'rejected')`, + ); + return { + data: (emptyResponse as unknown) as ResponseT, + // hack: set the http status to 204 no content so we can more + // easily track when no quote is returned + status: 204, + }; } const parsedResponse = diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index d8ad697a84..db5571178a 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -10,8 +10,8 @@ import { AltRfqMakerAssetOfferings, LogFunction, MarketOperation, - RfqPairType, RfqMakerAssetOfferings, + RfqPairType, RfqRequestOpts, SignedNativeOrder, TypedMakerUrl, @@ -455,6 +455,7 @@ export class QuoteRequestor { options.altRfqAssetOfferings || {}, requestParams, this._quoteRequestorHttpClient, + this._warningLogger, cancelTokenSource.token, ); diff --git a/packages/asset-swapper/test/quote_requestor_test.ts b/packages/asset-swapper/test/quote_requestor_test.ts index 1ef60a5fa4..e139cbc033 100644 --- a/packages/asset-swapper/test/quote_requestor_test.ts +++ b/packages/asset-swapper/test/quote_requestor_test.ts @@ -40,6 +40,8 @@ const ALT_RFQ_CREDS = { altRfqProfile: ALT_PROFILE, }; +const CREATED_STATUS_CODE = 201; + function makeThreeMinuteExpiry(): BigNumber { const expiry = new Date(Date.now()); expiry.setMinutes(expiry.getMinutes() + 3); @@ -188,7 +190,7 @@ describe('QuoteRequestor', async () => { altMockedRequests.push({ endpoint: 'https://132.0.0.1', mmApiKey: ALT_MM_API_KEY, - responseCode: StatusCodes.Success, + responseCode: CREATED_STATUS_CODE, requestData: altFirmRequestData, responseData: altFirmResponse, }); @@ -566,7 +568,7 @@ describe('QuoteRequestor', async () => { altMockedRequests.push({ endpoint: 'https://132.0.0.1', mmApiKey: ALT_MM_API_KEY, - responseCode: StatusCodes.Success, + responseCode: CREATED_STATUS_CODE, requestData: buyAmountAltRequest, responseData: buyAmountAltResponse, }); @@ -612,7 +614,7 @@ describe('QuoteRequestor', async () => { altMockedRequests.push({ endpoint: 'https://132.0.0.1', mmApiKey: ALT_MM_API_KEY, - responseCode: StatusCodes.Success, + responseCode: CREATED_STATUS_CODE, requestData: buyValueAltRequest, responseData: buyValueAltResponse, }); @@ -658,7 +660,7 @@ describe('QuoteRequestor', async () => { altMockedRequests.push({ endpoint: 'https://132.0.0.1', mmApiKey: ALT_MM_API_KEY, - responseCode: StatusCodes.Success, + responseCode: CREATED_STATUS_CODE, requestData: sellAmountAltRequest, responseData: sellAmountAltResponse, }); @@ -704,7 +706,7 @@ describe('QuoteRequestor', async () => { altMockedRequests.push({ endpoint: 'https://132.0.0.1', mmApiKey: ALT_MM_API_KEY, - responseCode: StatusCodes.Success, + responseCode: CREATED_STATUS_CODE, requestData: sellValueAltRequest, responseData: sellValueAltResponse, });