Skip to content

Commit

Permalink
feat: improve error message in case of 401 after the retries in the f…
Browse files Browse the repository at this point in the history
…irst 5 seconds (#168)
  • Loading branch information
gr2m authored Sep 14, 2020
1 parent fb13fa2 commit bc1f5d6
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export async function hook(
/**
* Newly created tokens might not be accessible immediately after creation.
* In case of a 401 response, we retry with an exponential delay until more
* than one minute passes since the creation of the token.
* than five seconds pass since the creation of the token.
*
* @see https://github.com/octokit/auth-app.js/issues/65
*/
Expand All @@ -69,13 +69,21 @@ async function sendRequestWithRetries(
}

if (timeSinceTokenCreationInMs >= FIVE_SECONDS_IN_MS) {
if (retries > 0) {
error.message = `After ${retries} retries within ${
timeSinceTokenCreationInMs / 1000
}s of creating the installation access token, the response remains 401. At this point, the cause may be an authentication problem or a system outage. Please check https://www.githubstatus.com for status information`;
}
throw error;
}

++retries;

const awaitTime = retries * 1000;
console.warn(
`[@octokit/auth-app] Retrying after 401 response to account for token replication delay (retry: ${retries}, wait: ${awaitTime}ms)`
`[@octokit/auth-app] Retrying after 401 response to account for token replication delay (retry: ${retries}, wait: ${
awaitTime / 1000
}s)`
);
await new Promise((resolve) => setTimeout(resolve, awaitTime));

Expand Down
57 changes: 57 additions & 0 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,63 @@ test("auth.hook(): handle 401 in first 5 seconds (#65)", async () => {
expect(global.console.warn.mock.calls.length).toEqual(3);
});

test("auth.hook(): throw error with custom message after unsuccessful retries (#163)", async () => {
expect.assertions(1);

const mock = fetchMock
.sandbox()
.postOnce("https://api.github.com/app/installations/123/access_tokens", {
token: "secret123",
expires_at: "1970-01-01T01:00:00.000Z",
permissions: {
metadata: "read",
},
repository_selection: "all",
})
.get("https://api.github.com/repos/octocat/hello-world", (url) => {
return {
status: 401,
body: {
message: "Bad credentials",
documentation_url: "https://developer.github.com/v3",
},
};
});

const auth = createAppAuth({
id: APP_ID,
privateKey: PRIVATE_KEY,
installationId: 123,
});

const requestWithMock = request.defaults({
headers: {
"user-agent": "test",
},
request: {
fetch: mock,
},
});
const requestWithAuth = requestWithMock.defaults({
request: {
hook: auth.hook,
},
});

global.console.warn = jest.fn();

requestWithAuth("GET /repos/octocat/hello-world").catch((error) => {
expect(error.message).toBe(
`After 3 retries within 6s of creating the installation access token, the response remains 401. At this point, the cause may be an authentication problem or a system outage. Please check https://www.githubstatus.com for status information`
);
});

// it takes 3 retries until a total time of more than 5s pass
await clock.tickAsync(1000);
await clock.tickAsync(2000);
await clock.tickAsync(3000);
});

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

0 comments on commit bc1f5d6

Please sign in to comment.