improve(PriceClient): Permit retry on failed lookups (#157)

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<TokenPrice> {

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<void> {
+    return new Promise((r) => setTimeout(r, ms));
   }
 }

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;
   }

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;
   }

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<unknown> {
+    return this.query(path, urlArgs);
+  }
+
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  protected override async sleep(_ms: number): Promise<void> {
+    ++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);

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(", ")})`);
     }
   }