From 4466b757da703a06a6407de0c90c49329ae9d89e Mon Sep 17 00:00:00 2001 From: Paul <108695806+pxrl@users.noreply.github.com> Date: Tue, 7 Mar 2023 18:57:27 +1100 Subject: [PATCH] improve(PriceClient): Permit retry on failed lookups (#157) --- src/priceClient/adapters/acrossApi.ts | 3 +- src/priceClient/adapters/baseAdapter.ts | 42 ++++++++++++--- src/priceClient/adapters/coingecko.ts | 3 +- src/priceClient/adapters/defiLlama.ts | 3 +- src/priceClient/priceClient.e2e.ts | 68 ++++++++++++++++++++++--- src/priceClient/priceClient.ts | 2 +- 6 files changed, 103 insertions(+), 18 deletions(-) diff --git a/src/priceClient/adapters/acrossApi.ts b/src/priceClient/adapters/acrossApi.ts index 7a1b579b..402f79ed 100644 --- a/src/priceClient/adapters/acrossApi.ts +++ b/src/priceClient/adapters/acrossApi.ts @@ -12,9 +12,10 @@ export class PriceFeed extends BaseHTTPAdapter implements PriceFeedAdapter { name = "Across API", host = "across.to", timeout = 5000, // ms + retries = 3, }: AcrossApiArgs = {}) { // Allow host to be overridden for test or alternative deployments. - super(name, host, { timeout }); + super(name, host, { timeout, retries }); } async getPriceByAddress(address: string, currency = "usd"): Promise { diff --git a/src/priceClient/adapters/baseAdapter.ts b/src/priceClient/adapters/baseAdapter.ts index a5fddab4..60f5a380 100644 --- a/src/priceClient/adapters/baseAdapter.ts +++ b/src/priceClient/adapters/baseAdapter.ts @@ -1,13 +1,24 @@ import assert from "assert"; -import axios, { AxiosError } from "axios"; +import axios from "axios"; export type BaseHTTPAdapterArgs = { timeout?: number; + retries?: number; }; export class BaseHTTPAdapter { + private _retries = 0; private _timeout = 0; + get retries(): number { + return this._retries; + } + + set retries(retries: number) { + assert(retries >= 0); + this._retries = retries; + } + get timeout(): number { return this._timeout; } @@ -17,7 +28,12 @@ export class BaseHTTPAdapter { this._timeout = timeout; } - constructor(public readonly name: string, public readonly host: string, { timeout = 1000 }: BaseHTTPAdapterArgs) { + constructor( + public readonly name: string, + public readonly host: string, + { timeout = 1000, retries = 1 }: BaseHTTPAdapterArgs + ) { + this.retries = retries; this.timeout = timeout; // ms } @@ -28,10 +44,22 @@ export class BaseHTTPAdapter { params: urlArgs ?? {}, }; - const result = await axios(url, args).catch((err) => { - const errMsg = err instanceof AxiosError ? err.message : err.toString(); - throw new Error(`${this.name} price lookup failure (${errMsg})`); - }); - return result.data; + const errs: string[] = []; + let tries = 0; + do { + try { + return (await axios(url, args)).data; + } catch (err) { + const errMsg = axios.isAxiosError(err) || err instanceof Error ? err.message : "unknown error"; + errs.push(errMsg); + if (++tries <= this.retries) await this.sleep(Math.pow(1.5, tries) * 1000); // simple backoff + } + } while (tries <= this.retries); + + throw new Error(`${this.name} price lookup failure (${errs.join(", ")})`); + } + + protected async sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); } } diff --git a/src/priceClient/adapters/coingecko.ts b/src/priceClient/adapters/coingecko.ts index 08398043..8c59f308 100644 --- a/src/priceClient/adapters/coingecko.ts +++ b/src/priceClient/adapters/coingecko.ts @@ -18,9 +18,10 @@ type CoinGeckoArgs = BaseHTTPAdapterArgs & { export class PriceFeed extends BaseHTTPAdapter implements PriceFeedAdapter { private readonly apiKey: string | undefined = undefined; - constructor({ name, apiKey, timeout = 5000 }: CoinGeckoArgs = {}) { + constructor({ name, apiKey, timeout = 5000, retries = 3 }: CoinGeckoArgs = {}) { super(name ?? apiKey ? "CoinGecko Pro" : "CoinGecko Free", apiKey ? "pro-api.coingecko.com" : "api.coingecko.com", { timeout, + retries, }); this.apiKey = apiKey; } diff --git a/src/priceClient/adapters/defiLlama.ts b/src/priceClient/adapters/defiLlama.ts index be547de3..22362358 100644 --- a/src/priceClient/adapters/defiLlama.ts +++ b/src/priceClient/adapters/defiLlama.ts @@ -28,9 +28,10 @@ export class PriceFeed extends BaseHTTPAdapter implements PriceFeedAdapter { name = "DefiLlama", host = "coins.llama.fi", timeout = 5000, + retries = 2, minConfidence = 0.9, }: DefiLlamaArgs = {}) { - super(name, host, { timeout }); + super(name, host, { timeout, retries }); assert(minConfidence >= 0.0 && minConfidence <= 1.0); this._minConfidence = minConfidence; } diff --git a/src/priceClient/priceClient.e2e.ts b/src/priceClient/priceClient.e2e.ts index 756185a7..2074dee4 100644 --- a/src/priceClient/priceClient.e2e.ts +++ b/src/priceClient/priceClient.e2e.ts @@ -1,11 +1,30 @@ import assert from "assert"; +import axios from "axios"; import dotenv from "dotenv"; import winston from "winston"; import { Logger, msToS, PriceCache, PriceClient, PriceFeedAdapter, TokenPrice } from "./priceClient"; import { acrossApi, coingecko, defiLlama } from "./adapters"; +import { BaseHTTPAdapter, BaseHTTPAdapterArgs } from "./adapters/baseAdapter"; dotenv.config(); +class TestBaseHTTPAdapter extends BaseHTTPAdapter { + public nRetries = 0; + + constructor(name: string, host: string, { timeout = 500, retries = 1 }: BaseHTTPAdapterArgs) { + super(name, host, { timeout, retries }); + } + + async _query(path: string, urlArgs?: object): Promise { + return this.query(path, urlArgs); + } + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + protected override async sleep(_ms: number): Promise { + ++this.nRetries; + } +} + class TestPriceClient extends PriceClient { constructor(logger: Logger, priceFeeds: PriceFeedAdapter[]) { super(logger, priceFeeds); @@ -69,6 +88,41 @@ function validateTokenPrice(tokenPrice: TokenPrice, address: string, timestamp: ); } +describe("PriceClient: BaseHTTPAdapter", function () { + test("Retry behaviour: All failures", async function () { + for (const retries of [0, 1, 3, 5, 7, 9]) { + const name = `BaseHTTPAdapter test w/ ${retries} retries`; + const baseAdapter = new TestBaseHTTPAdapter(name, "127.0.0.1", { timeout: 1, retries }); + expect(baseAdapter.nRetries).toBe(0); + await expect(baseAdapter._query("", { retries })).rejects.toThrow(`${name} price lookup failure`); + expect(baseAdapter.nRetries).toBe(retries); + } + }); + + test("Retry behaviour: Success on the final request", async function () { + for (const retries of [1, 3, 5, 7, 9]) { + const name = `BaseHTTPAdapter test w/ success on retry ${retries}`; + const baseAdapter = new TestBaseHTTPAdapter(name, "127.0.0.1", { timeout: 1, retries }); + expect(baseAdapter.nRetries).toBe(0); + + // Instantiate callback for HTTP response != 2xx. + const interceptor = axios.interceptors.response.use( + undefined, // HTTP 2xx. + function (error) { + const result = retries && baseAdapter.nRetries === retries ? Promise.resolve({}) : Promise.reject(error); + return result; + } + ); + + const response = baseAdapter._query("", { retries }); + await expect(response).resolves.not.toThrow(); + axios.interceptors.response.eject(interceptor); // Cleanup ASAP. + + expect(baseAdapter.nRetries).toBe(retries); + } + }); +}); + // this requires e2e testing, should only test manually for now describe("PriceClient", function () { const dummyLogger: winston.Logger = winston.createLogger({ @@ -140,7 +194,7 @@ describe("PriceClient", function () { test("getPriceByAddress: Across failover to Across", async function () { pc = new PriceClient(dummyLogger, [ - new acrossApi.PriceFeed({ name: "Across API (expect fail)", host: "127.0.0.1" }), + new acrossApi.PriceFeed({ name: "Across API (expect fail)", host: "127.0.0.1", retries: 0 }), new acrossApi.PriceFeed({ name: "Across API (expect pass)" }), ]); @@ -151,7 +205,7 @@ describe("PriceClient", function () { test("getPriceByAddress: Coingecko failover to Across", async function () { const _apiKey = "xxx-fake-apikey"; pc = new PriceClient(dummyLogger, [ - new coingecko.PriceFeed({ name: "CoinGecko Pro (expect fail)", apiKey: _apiKey }), + new coingecko.PriceFeed({ name: "CoinGecko Pro (expect fail)", apiKey: _apiKey, retries: 0 }), new acrossApi.PriceFeed({ name: "Across API (expect pass)" }), ]); @@ -161,14 +215,14 @@ describe("PriceClient", function () { test("getPriceByAddress: Complete price lookup failure", async function () { pc = new PriceClient(dummyLogger, [ - new acrossApi.PriceFeed({ name: "Across API #1 (expect fail)", host: "127.0.0.1" }), - new acrossApi.PriceFeed({ name: "Across API #2 (expect fail)", host: "127.0.0.1" }), + new acrossApi.PriceFeed({ name: "Across API #1 (expect fail)", host: "127.0.0.1", retries: 0 }), + new acrossApi.PriceFeed({ name: "Across API #2 (expect fail)", host: "127.0.0.1", retries: 0 }), ]); await expect(pc.getPriceByAddress(testAddress)).rejects.toThrow(); }); test("getPriceByAddress: Across API timeout", async function () { - const acrossPriceFeed: acrossApi.PriceFeed = new acrossApi.PriceFeed({ name: "Across API (timeout)" }); + const acrossPriceFeed: acrossApi.PriceFeed = new acrossApi.PriceFeed({ name: "Across API (timeout)", retries: 0 }); pc = new PriceClient(dummyLogger, [acrossPriceFeed]); acrossPriceFeed.timeout = 1; // mS @@ -206,7 +260,7 @@ describe("PriceClient", function () { test("getPriceByAddress: Address case insensitivity", async function () { // Instantiate a custom subclass of PriceClient. const pc: TestPriceClient = new TestPriceClient(dummyLogger, [ - new acrossApi.PriceFeed({ name: "Across API (expect fail)", host: "127.0.0.1" }), + new acrossApi.PriceFeed({ name: "Across API (expect fail)", host: "127.0.0.1", retries: 0 }), ]); // Load the cache with lower-case addresses, then query with an upper-case address. @@ -234,7 +288,7 @@ describe("PriceClient", function () { test("getPriceByAddress: Validate price cache", async function () { // Instantiate a custom subclass of PriceClient; load the cache and force price lookup failures. const pc: TestPriceClient = new TestPriceClient(dummyLogger, [ - new acrossApi.PriceFeed({ name: "Across API (expect fail)", host: "127.0.0.1" }), + new acrossApi.PriceFeed({ name: "Across API (expect fail)", host: "127.0.0.1", retries: 0 }), ]); const priceCache: PriceCache = pc.getProtectedPriceCache(baseCurrency); diff --git a/src/priceClient/priceClient.ts b/src/priceClient/priceClient.ts index 6815c04d..053d3d64 100644 --- a/src/priceClient/priceClient.ts +++ b/src/priceClient/priceClient.ts @@ -145,7 +145,7 @@ export class PriceClient implements PriceFeedAdapter { priceFeeds: this.listPriceFeeds(), tokens: addresses, }); - throw Error(`Price lookup failed against all price feeds (${this.listPriceFeeds().join(", ")})`); + throw new Error(`Price lookup failed against all price feeds (${this.listPriceFeeds().join(", ")})`); } }