Add option to provide a custom predicate to decide whether to retry

README.md

@@ -73,6 +73,22 @@ const octokit = new MyOctokit({
 });
 ```
 
+You can override the default predicate that determines whether to retry a request based on the outcome of the previous attempt. For example, to retry if the response message includes a particular string. Note that the `doNotRetry` option from the constructor is ignored in this case.
+
+```typescript
+const octokit = new MyOctokit({
+  auth: "secret123",
+  retry: {
+    shouldRetry: (retryState: RetryState, error: any) => {
+      if (isRequestError(error)) {
+        return error.message.includes("Intermittent problem");
+      }
+      return false;
+    },
+  },
+});
+```
+
 To override the number of retries:
 
 ```js

src/error-request.ts

@@ -1,28 +1,45 @@
-import { type RetryPlugin, type RetryState } from "./types.js";
-import type { RequestRequestOptions } from "@octokit/types";
-import type { RequestError } from "@octokit/request-error";
+import {
+  type RequestOptionsWithRequest,
+  type RetryPlugin,
+  type RetryState,
+} from "./types.js";
+import { RequestError } from "@octokit/request-error";
 
 export function isRequestError(error: any): error is RequestError {
   return error.request !== undefined;
 }
 
-export async function errorRequest(
+export function defaultShouldRetry(
   state: RetryState,
-  octokit: RetryPlugin,
   error: RequestError | Error,
-  options: { request: RequestRequestOptions },
-): Promise<any> {
+): boolean {
   if (!isRequestError(error) || !error?.request.request) {
     // address https://github.com/octokit/plugin-retry.js/issues/8
     throw error;
   }
 
-  // retry all >= 400 && not doNotRetry
-  if (error.status >= 400 && !state.doNotRetry.includes(error.status)) {
-    const retries =
-      options.request.retries != null ? options.request.retries : state.retries;
-    const retryAfter = Math.pow((options.request.retryCount || 0) + 1, 2);
-    throw octokit.retry.retryRequest(error, retries, retryAfter);
+  return error.status >= 400 && !state.doNotRetry.includes(error.status);
+}
+
+export async function errorRequest(
+  state: RetryState,
+  octokit: RetryPlugin,
+  error: RequestError | Error,
+  options: RequestOptionsWithRequest,
+): Promise<any> {
+  if (state.shouldRetry(state, error)) {
+    const retries: number =
+      options.request?.retries != null
+        ? options.request.retries
+        : state.retries;
+    const retryAfter = Math.pow((options.request?.retryCount || 0) + 1, 2);
+    throw new RequestError(
+      error.message,
+      isRequestError(error) ? error.status : 500,
+      {
+        request: octokit.retry.retryRequest(options, retries, retryAfter),
+      },
+    );
   }
 
   // Maybe eventually there will be more cases here

src/index.ts

@@ -1,40 +1,50 @@
 import type { Octokit, OctokitOptions } from "@octokit/core";
-import type { RequestError } from "@octokit/request-error";
 
 import { VERSION } from "./version.js";
-import { errorRequest } from "./error-request.js";
+import { defaultShouldRetry, errorRequest } from "./error-request.js";
 import { wrapRequest } from "./wrap-request.js";
-import type { RetryOptions, RetryPlugin, RetryState } from "./types.js";
+import type {
+  RequestOptionsWithRequest,
+  RetryOptions,
+  RetryPlugin,
+  RetryRequestOptions,
+  RetryState,
+} from "./types.js";
 import type { RequestRequestOptions } from "@octokit/types";
 export { VERSION } from "./version.js";
 
+export const defaultRetryState: RetryState = {
+  enabled: true,
+  retryAfterBaseValue: 1000,
+  doNotRetry: [400, 401, 403, 404, 410, 422, 451],
+  retries: 3,
+  shouldRetry: defaultShouldRetry,
+};
+
 export function retry(
   octokit: Octokit,
   octokitOptions: OctokitOptions,
 ): RetryPlugin {
   const state: RetryState = Object.assign(
-    {
-      enabled: true,
-      retryAfterBaseValue: 1000,
-      doNotRetry: [400, 401, 403, 404, 410, 422, 451],
-      retries: 3,
-    } satisfies RetryState,
+    {},
+    defaultRetryState,
     octokitOptions.retry,
   );
 
   const retryPlugin: RetryPlugin = {
     retry: {
       retryRequest: (
-        error: RequestError,
+        request: RequestOptionsWithRequest,
         retries: number,
         retryAfter: number,
       ) => {
-        error.request.request = Object.assign({}, error.request.request, {
+        const newRequest: RequestRequestOptions &
+          Required<RetryRequestOptions> = Object.assign({}, request.request, {
           retries: retries,
           retryAfter: retryAfter,
-        } satisfies RequestRequestOptions);
+        } as Required<RetryRequestOptions>);
 
-        return error;
+        return { ...request, request: newRequest };
       },
     },
   };
@@ -54,4 +64,11 @@ declare module "@octokit/core/types" {
   }
 }
 
+declare module "@octokit/types" {
+  interface RequestRequestOptions {
+    retries?: number;
+    retryAfter?: number;
+  }
+}
+
 export type { RetryPlugin, RetryOptions };

src/types.ts

@@ -1,12 +1,24 @@
 import type { RequestError } from "@octokit/request-error";
+import type { RequestOptions, RequestRequestOptions } from "@octokit/types";
+
+export interface RetryRequestOptions {
+  retries?: number;
+  retryAfter?: number;
+}
+
+export type RequestOptionsWithRequest = RequestOptions & {
+  request: RequestRequestOptions & RetryRequestOptions;
+};
 
 export interface RetryPlugin {
   retry: {
     retryRequest: (
-      error: RequestError,
+      request: RequestOptionsWithRequest,
       retries: number,
       retryAfter: number,
-    ) => RequestError;
+    ) => RequestOptions & {
+      request: RequestRequestOptions & Required<RetryRequestOptions>;
+    };
   };
 }
 
@@ -15,6 +27,7 @@ export interface RetryOptions {
   retryAfterBaseValue?: number;
   doNotRetry?: number[];
   retries?: number;
+  shouldRetry?: (state: RetryState, error: RequestError | Error) => boolean;
 }
 
 export type RetryState = Required<RetryOptions>;

src/wrap-request.ts

@@ -17,15 +17,18 @@ export async function wrapRequest(
   const limiter = new Bottleneck();
 
   limiter.on("failed", function (error: RequestError, info: RetryableInfo) {
-    const maxRetries = ~~error.request.request?.retries;
-    const after = ~~error.request.request?.retryAfter;
+    const maxRetries = error.request.request?.retries || 0;
+    const after = error.request.request?.retryAfter || 0;
     options.request.retryCount = info.retryCount + 1;
 
     if (maxRetries > info.retryCount) {
       // Returning a number instructs the limiter to retry
       // the request after that number of milliseconds have passed
       return after * state.retryAfterBaseValue;
     }
+
+    // Do not retry.
+    return undefined;
   });
 
   return limiter.schedule(

test/error-request.test.ts

@@ -0,0 +1,92 @@
+import { describe, it, expect } from "vitest";
+import { defaultShouldRetry, errorRequest } from "../src/error-request.ts";
+import { defaultRetryState } from "../src/index.ts";
+import { RequestError } from "@octokit/request-error";
+import { TestOctokit } from "./octokit.ts";
+import { RetryState } from "../src/types.ts";
+import type { RequestMethod, RequestOptions } from "@octokit/types";
+
+describe("defaultShouldRetry", function () {
+  it("should re-throw non-RequestError errors", function () {
+    try {
+      defaultShouldRetry(defaultRetryState, new Error("Re-throw me"));
+      throw new Error("Should not reach this point");
+    } catch (err: any) {
+      expect(err.message).toEqual("Re-throw me");
+    }
+  });
+
+  it("should re-throw errors without RequestRequestOptions", function () {
+    try {
+      defaultShouldRetry(
+        defaultRetryState,
+        new RequestError("Re-throw me", 500, {
+          request: { method: "GET", url: "/something", headers: {} },
+        }),
+      );
+      throw new Error("Should not reach this point");
+    } catch (err: any) {
+      expect(err.message).toEqual("Re-throw me");
+    }
+  });
+
+  it("returns false for doNotRetry status codes", function () {
+    for (const statusCode of defaultRetryState.doNotRetry) {
+      const result = defaultShouldRetry(
+        defaultRetryState,
+        new RequestError("Re-throw me", statusCode, {
+          request: {
+            method: "GET",
+            url: "/something",
+            headers: {},
+            request: {},
+          },
+        }),
+      );
+      expect(result).toBe(false);
+    }
+  });
+
+  it("returns true for 500 errors", function () {
+    const result = defaultShouldRetry(
+      defaultRetryState,
+      new RequestError("Re-throw me", 500, {
+        request: {
+          method: "GET",
+          url: "/something",
+          headers: {},
+          request: {},
+        },
+      }),
+    );
+    expect(result).toBe(true);
+  });
+});
+
+describe("errorRequest", function () {
+  it("allows non-RequestErrors to be retried", async function () {
+    const state: RetryState = {
+      ...defaultRetryState,
+      shouldRetry: (_state, _error) => true,
+    };
+    const requestOptions = {
+      method: "GET" as RequestMethod,
+      url: "/issues",
+      headers: {},
+      request: {},
+    } satisfies RequestOptions;
+
+    try {
+      await errorRequest(
+        state,
+        new TestOctokit(),
+        new Error("Some non-RequestError"),
+        requestOptions,
+      );
+      throw new Error("Should not reach this point");
+    } catch (error: any) {
+      expect(error.message).toBe("Some non-RequestError");
+      expect(error.request.request.retries).toBe(3);
+    }
+  });
+});

test/retry.test.ts

@@ -1,6 +1,10 @@
 import { describe, it, expect } from "vitest";
 import { TestOctokit } from "./octokit.ts";
-import { errorRequest, isRequestError } from "../src/error-request.ts";
+import {
+  defaultShouldRetry,
+  errorRequest,
+  isRequestError,
+} from "../src/error-request.ts";
 import { RequestError } from "@octokit/request-error";
 import type {
   RequestMethod,
@@ -58,6 +62,43 @@ describe("Automatic Retries", function () {
     ).toBeLessThan(20);
   });
 
+  it("Should retry and pass with custom shouldRetry function", async function () {
+    const octokit = new TestOctokit({
+      shouldRetry: (_retryState: RetryState, error: any) => {
+        if (isRequestError(error)) {
+          return error.message.includes("Intermittent problem");
+        }
+        return false;
+      },
+    });
+
+    const response = await octokit.request("GET /route", {
+      request: {
+        responses: [
+          {
+            status: 403,
+            headers: {},
+            data: { message: "Intermittent problem" },
+          },
+          { status: 200, headers: {}, data: { message: "Success!" } },
+        ],
+        retries: 1,
+      },
+    });
+
+    expect(response.status).toEqual(200);
+    expect(response.data).toStrictEqual({ message: "Success!" });
+    expect(octokit.__requestLog).toStrictEqual([
+      "START GET /route",
+      "START GET /route",
+      "END GET /route",
+    ]);
+
+    expect(
+      octokit.__requestTimings[1] - octokit.__requestTimings[0],
+    ).toBeLessThan(20);
+  });
+
   it("Should retry twice and fail", async function () {
     const octokit = new TestOctokit();
 
@@ -414,6 +455,7 @@ describe("errorRequest", function () {
       retryAfterBaseValue: 1000,
       doNotRetry: [400, 401, 403, 404, 422],
       retries: 3,
+      shouldRetry: defaultShouldRetry,
     } satisfies RetryState;
     const requestOptions = {
       method: "GET" as RequestMethod,
@@ -448,6 +490,7 @@ describe("errorRequest", function () {
       retryAfterBaseValue: 1000,
       doNotRetry: [400, 401, 403, 404, 422],
       retries: 3,
+      shouldRetry: defaultShouldRetry,
     } satisfies RetryState;
     const requestOptions = {
       method: "GET" as RequestMethod,