diff --git a/README.md b/README.md index 1b5eeb9a..99a956ec 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,19 @@ const octokit = new MyOctokit({ }); ``` +To override the retry strategy: + +```js +const octokit = new MyOctokit({ + auth: "secret123", + retry: { + // retry strategy, can be one of `exponential`, `linear`, or `polynomial` + // defaults to `polynomial` + strategy: "polynomial", + }, +}); +``` + You can manually ask for retries for any request by passing `{ request: { retries: numRetries, retryAfter: delayInSeconds }}`. Note that the `doNotRetry` option from the constructor is ignored in this case, requests will be retried no matter their response code. ```js diff --git a/src/error-request.ts b/src/error-request.ts index da4a7a5d..0b20b914 100644 --- a/src/error-request.ts +++ b/src/error-request.ts @@ -10,7 +10,19 @@ export async function errorRequest(state, octokit, error, options) { 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); + const retryCount = (options.request.retryCount || 0) + 1; + let retryAfter; + switch (state.strategy) { + case "exponential": + retryAfter = Math.round(Math.pow(2, retryCount)); + break; + case "linear": + retryAfter = retryCount; + break; + default: // "polynomial" + retryAfter = Math.round(Math.pow(retryCount, 2)); + break; + } throw octokit.retry.retryRequest(error, retries, retryAfter); } diff --git a/src/index.ts b/src/index.ts index f2d8bf9b..577175ed 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,6 +6,12 @@ import { wrapRequest } from "./wrap-request"; export const VERSION = "0.0.0-development"; +export enum RetryStrategy { + exponential = "exponential", + linear = "linear", + polynomial = "polynomial", +} + export function retry(octokit: Octokit, octokitOptions: any) { const state = Object.assign( { @@ -13,10 +19,15 @@ export function retry(octokit: Octokit, octokitOptions: any) { retryAfterBaseValue: 1000, doNotRetry: [400, 401, 403, 404, 422], retries: 3, + strategy: RetryStrategy.polynomial, }, octokitOptions.retry ); + if (!RetryStrategy.hasOwnProperty(state.strategy)) { + throw new Error(`Invalid retry strategy: ${state.strategy}`); + } + if (state.enabled) { octokit.hook.error("request", errorRequest.bind(null, state, octokit)); octokit.hook.wrap("request", wrapRequest.bind(null, state, octokit)); diff --git a/test/retry.test.ts b/test/retry.test.ts index 690d9a87..fae07224 100644 --- a/test/retry.test.ts +++ b/test/retry.test.ts @@ -3,6 +3,14 @@ import { errorRequest } from "../src/error-request"; import { RequestError } from "@octokit/request-error"; import { RequestMethod } from "@octokit/types"; +describe("Octokit", function () { + it("Should fail on creation if strategy is invalid", function () { + expect(() => { + new TestOctokit({ retry: { strategy: "invalid" } }); + }).toThrow("Invalid retry strategy: invalid"); + }); +}); + describe("Automatic Retries", function () { it("Should be possible to disable the plugin", async function () { const octokit = new TestOctokit({ retry: { enabled: false } }); @@ -164,7 +172,7 @@ describe("Automatic Retries", function () { expect(ms).toBeLessThan(45); }); - it("Should trigger exponential retries on HTTP 500 errors", async function () { + it("Should trigger polynomial retries on HTTP 500 errors", async function () { const octokit = new TestOctokit({ retry: { retryAfterBaseValue: 50 } }); const res = await octokit.request("GET /route", { @@ -202,7 +210,7 @@ describe("Automatic Retries", function () { const ms2 = octokit.__requestTimings[3] - octokit.__requestTimings[2]; expect(ms2).toBeLessThan(470); - expect(ms2).toBeGreaterThan(420); + expect(ms2).toBeGreaterThan(430); }); it("Should not retry 3xx/400/401/403/422 errors", async function () { @@ -266,12 +274,104 @@ describe("Automatic Retries", function () { expect(caught).toEqual(testStatuses.length); }); + it("Should allow to override the strategy with exponential", async function () { + const octokit = new TestOctokit({ + retry: { + strategy: "exponential", + retryAfterBaseValue: 50, + }, + }); + + const res = await octokit.request("GET /route", { + request: { + responses: [ + { status: 500, headers: {}, data: { message: "Did not retry, one" } }, + { status: 500, headers: {}, data: { message: "Did not retry, two" } }, + { + status: 500, + headers: {}, + data: { message: "Did not retry, three" }, + }, + { status: 200, headers: {}, data: { message: "Success!" } }, + ], + }, + }); + + expect(res.status).toEqual(200); + expect(res.data).toStrictEqual({ message: "Success!" }); + expect(octokit.__requestLog).toStrictEqual([ + "START GET /route", + "START GET /route", + "START GET /route", + "START GET /route", + "END GET /route", + ]); + + const ms0 = octokit.__requestTimings[1] - octokit.__requestTimings[0]; + expect(ms0).toBeLessThan(120); + expect(ms0).toBeGreaterThan(80); + + const ms1 = octokit.__requestTimings[2] - octokit.__requestTimings[1]; + expect(ms1).toBeLessThan(220); + expect(ms1).toBeGreaterThan(180); + + const ms2 = octokit.__requestTimings[3] - octokit.__requestTimings[2]; + expect(ms2).toBeLessThan(420); + expect(ms2).toBeGreaterThan(380); + }); + + it("Should allow to override the strategy with linear", async function () { + const octokit = new TestOctokit({ + retry: { + strategy: "linear", + retryAfterBaseValue: 50, + }, + }); + + const res = await octokit.request("GET /route", { + request: { + responses: [ + { status: 500, headers: {}, data: { message: "Did not retry, one" } }, + { status: 500, headers: {}, data: { message: "Did not retry, two" } }, + { + status: 500, + headers: {}, + data: { message: "Did not retry, three" }, + }, + { status: 200, headers: {}, data: { message: "Success!" } }, + ], + }, + }); + + expect(res.status).toEqual(200); + expect(res.data).toStrictEqual({ message: "Success!" }); + expect(octokit.__requestLog).toStrictEqual([ + "START GET /route", + "START GET /route", + "START GET /route", + "START GET /route", + "END GET /route", + ]); + + const ms0 = octokit.__requestTimings[1] - octokit.__requestTimings[0]; + expect(ms0).toBeLessThan(70); + expect(ms0).toBeGreaterThan(30); + + const ms1 = octokit.__requestTimings[2] - octokit.__requestTimings[1]; + expect(ms1).toBeLessThan(120); + expect(ms1).toBeGreaterThan(80); + + const ms2 = octokit.__requestTimings[3] - octokit.__requestTimings[2]; + expect(ms2).toBeLessThan(170); + expect(ms2).toBeGreaterThan(130); + }); + it('Should retry "Something went wrong" GraphQL error', async function () { const octokit = new TestOctokit(); const result = await octokit.graphql({ - query: `query { - viewer { + query: `query { + viewer { login } }`, @@ -347,8 +447,8 @@ describe("Automatic Retries", function () { try { await octokit.graphql({ - query: `query { - viewer { + query: `query { + viewer { login } }`,