Skip to content

Commit

Permalink
fix: reduce the retry time to account for replication lag from 1 minu…
Browse files Browse the repository at this point in the history
…te to 5 seconds (#162)
  • Loading branch information
gr2m committed Sep 1, 2020
1 parent 20148cd commit 22b88b7
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 16 deletions.
6 changes: 3 additions & 3 deletions src/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
State,
} from "./types";

const SIXTY_SECONDS_IN_MS = 60 * 1000;
const FIVE_SECONDS_IN_MS = 5 * 1000;

export async function hook(
state: State,
Expand Down Expand Up @@ -68,12 +68,12 @@ async function sendRequestWithRetries(
throw error;
}

if (timeSinceTokenCreationInMs >= SIXTY_SECONDS_IN_MS) {
if (timeSinceTokenCreationInMs >= FIVE_SECONDS_IN_MS) {
throw error;
}

++retries;
const awaitTime = retries * retries * 1000;
const awaitTime = retries * 1000;
console.warn(
`[@octokit/auth-app] Retrying after 401 response to account for token replication delay (retry: ${retries}, wait: ${awaitTime}ms)`
);
Expand Down
20 changes: 7 additions & 13 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1210,9 +1210,8 @@ test("auth.hook() uses app auth for marketplace URL", async () => {
expect(mock.done()).toBe(true);
});

test("auth.hook(): handle 401 in first minute (#65)", async () => {
let requestCount = 0;
const ONE_MINUTE_IN_MS = 1000 * 60;
test("auth.hook(): handle 401 in first 5 seconds (#65)", async () => {
const FIVE_SECONDS_IN_MS = 1000 * 5;

const mock = fetchMock
.sandbox()
Expand All @@ -1225,7 +1224,7 @@ test("auth.hook(): handle 401 in first minute (#65)", async () => {
repository_selection: "all",
})
.get("https://api.github.com/repos/octocat/hello-world", (url) => {
if (Date.now() < ONE_MINUTE_IN_MS) {
if (Date.now() < FIVE_SECONDS_IN_MS) {
return {
status: 401,
body: {
Expand Down Expand Up @@ -1280,15 +1279,10 @@ test("auth.hook(): handle 401 in first minute (#65)", async () => {

const promise = requestWithAuth("GET /repos/octocat/hello-world");

let i = 0;

// it takes 6 retries until a total time of more than 60s pass
// it takes 3 retries until a total time of more than 5s pass
await clock.tickAsync(1000);
await clock.tickAsync(4000);
await clock.tickAsync(9000);
await clock.tickAsync(16000);
await clock.tickAsync(25000);
await clock.tickAsync(36000);
await clock.tickAsync(2000);
await clock.tickAsync(3000);

const { data } = await promise;

Expand All @@ -1303,7 +1297,7 @@ test("auth.hook(): handle 401 in first minute (#65)", async () => {
expect(mock.done()).toBe(true);

// @ts-ignore
expect(global.console.warn.mock.calls.length).toEqual(6);
expect(global.console.warn.mock.calls.length).toEqual(3);
});

test("auth.hook(): throws on 500 error without retries", async () => {
Expand Down

0 comments on commit 22b88b7

Please sign in to comment.