Compute retry delay correctly when both useExponentialBackoff and retryDelay are specified (#398)

Author: taylorreece

packages/spectral/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@prismatic-io/spectral",
-  "version": "10.16.0",
+  "version": "10.16.1",
   "description": "Utility library for building Prismatic connectors and code-native integrations",
   "keywords": ["prismatic"],
   "main": "dist/index.js",

packages/spectral/src/clients/http/index.ts

@@ -61,7 +61,10 @@ interface RetryConfig extends Omit<IAxiosRetryConfig, "retryDelay"> {
    * a retryCondition function to determine when retries should occur.
    */
   retryAllErrors?: boolean;
-  /** When true, double the retry delay after each attempt (e.g. 1000ms, 2000ms, 4000ms, 8000ms, etc.). */
+  /**
+   * When true, double the retryDelay after each attempt (e.g. 1000ms, 2000ms, 4000ms, 8000ms, etc.).
+   * If no retryDelay is specified, defaults to axios-retry's exponentialDelay function.
+   */
   useExponentialBackoff?: boolean;
 }
 
@@ -87,12 +90,15 @@ const computeRetryDelay = (
   useExponentialBackoff: RetryConfig["useExponentialBackoff"],
 ): IAxiosRetryConfig["retryDelay"] => {
   if (useExponentialBackoff) {
-    return exponentialDelay;
+    return typeof retryDelay === "number"
+      ? (retryCount) => 2 ** (retryCount - 1) * retryDelay
+      : exponentialDelay;
   }
+  // retryDelay is either a number or a function
   return typeof retryDelay === "number" ? () => retryDelay : retryDelay;
 };
 
-const toAxiosRetryConfig = ({
+export const toAxiosRetryConfig = ({
   retryDelay,
   retryAllErrors,
   retryCondition,

packages/spectral/src/clients/http/retry.test.ts

@@ -0,0 +1,45 @@
+import { describe, expect } from "vitest";
+import { test } from "@fast-check/vitest";
+import type { AxiosError } from "axios";
+import { toAxiosRetryConfig } from ".";
+
+describe("Ensure retry configuration functions work as expected", () => {
+  test("Delay function returns fixed delay when useExponentialBackoff is false", () => {
+    const retryDelay = 1234;
+    const axiosConfig = toAxiosRetryConfig({
+      retries: 3,
+      retryDelay,
+      useExponentialBackoff: false,
+    });
+    const delayFunction = axiosConfig.retryDelay;
+    expect(delayFunction?.(3, {} as AxiosError)).toEqual(retryDelay);
+  });
+  test("Delay function returns retryDelay when useExponentialBackoff is true on first retry", () => {
+    const retryDelay = 1234;
+    const axiosConfig = toAxiosRetryConfig({ retries: 3, retryDelay, useExponentialBackoff: true });
+    const delayFunction = axiosConfig.retryDelay;
+    expect(delayFunction?.(1, {} as AxiosError)).toEqual(retryDelay);
+  });
+  test("Delay function returns retryDelay * 2 * 2 when useExponentialBackoff is true on fourth retry", () => {
+    const retryDelay = 1234;
+    const axiosConfig = toAxiosRetryConfig({ retries: 4, retryDelay, useExponentialBackoff: true });
+    const delayFunction = axiosConfig.retryDelay;
+    expect(delayFunction?.(4, {} as AxiosError)).toEqual(retryDelay * 2 ** 3);
+  });
+  test("Delay function returns exponential delay when useExponentialBackoff is true and retryDelay is not set", () => {
+    const axiosConfig = toAxiosRetryConfig({ retries: 3, useExponentialBackoff: true });
+    const delayFunction = axiosConfig.retryDelay;
+    const delay1 = delayFunction?.(1, {} as AxiosError);
+    const delay2 = delayFunction?.(2, {} as AxiosError);
+    const delay3 = delayFunction?.(3, {} as AxiosError);
+
+    // By default, axios-retry's exponentialDelay function returns 200 (+20% random jitter) on first retry,
+    // 400 (+20% random jitter) on second retry, 800 (+20% random jitter) on third retry, etc.
+    expect(delay1).toBeGreaterThanOrEqual(100 * 2);
+    expect(delay1).toBeLessThanOrEqual(100 * 2 * 1.2);
+    expect(delay2).toBeGreaterThanOrEqual(100 * 4);
+    expect(delay2).toBeLessThanOrEqual(100 * 4 * 1.2);
+    expect(delay3).toBeGreaterThanOrEqual(100 * 8);
+    expect(delay3).toBeLessThanOrEqual(100 * 8 * 1.2);
+  });
+});