feat: improve error message in case of 401 after the retries in the first 5 seconds #168
Conversation
test/index.test.ts
Outdated
|
|
||
| requestWithAuth("GET /repos/octocat/hello-world").catch((error) => { | ||
| expect(error.message).toBe( | ||
| `[@octokit/auth-app] After 3 retries within 6s after creating the installation access token, the response remains 401. At this point it's unlikely a replication lag, but a legit authentication problem or a system outage, please check https://www.githubstatus.com/` |
There was a problem hiding this comment.
@gwestersf @imwiss This is the error message people will see when the request retries fail to account for the installation access token replication lag. Let me know if you'd like me to rephrase it in any way.
This is just the message, the error has extra properties by default with more information.
RequestError [HttpError]: [@octokit/auth-app] After 3 retries within 6s after creating the installation access token, the response remains 401. At this point it's unlikely a replication lag, but a legit authentication problem or a system outage, please check https://www.githubstatus.com/
at /Users/gregor/Projects/octokit/auth-app.js/node_modules/@octokit/request/dist-src/fetch-wrapper.js:49:31
at sendRequestWithRetries (/Users/gregor/Projects/octokit/auth-app.js/src/hook.ts:65:12) {
status: 401,
headers: { 'content-length': '83', 'content-type': 'application/json' },
request: {
method: 'GET',
url: 'https://api.github.com/repos/octocat/hello-world',
headers: {
accept: 'application/vnd.github.v3+json',
'user-agent': 'test',
authorization: 'token [REDACTED]'
},
},
documentation_url: 'https://developer.github.com/v3'
}
There was a problem hiding this comment.
Thanks for the ping 👍 Additionally, are there any error messages returned by the API with the 401 that would make sense to surface in Octokit's error message?
There was a problem hiding this comment.
Formatting suggestion:
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.
There was a problem hiding this comment.
are there any error messages returned by the API with the 401 that would make sense to surface in Octokit's error message
No. The only other information is documentation_url which we add to the error object.
| error.message = `[@octokit/auth-app] After ${retries} retries within ${ | ||
| timeSinceTokenCreationInMs / 1000 | ||
| }s after creating the installation access token, the response remains 401. At this point it's unlikely a replication lag, but a legit authentication problem or a system outage, please check https://www.githubstatus.com/`; | ||
| } |
There was a problem hiding this comment.
this is the only relevant change, the changes below are just reformatting for readability
There was a problem hiding this comment.
The comment on line 52 should be updated to 5 seconds instead of 1 minute.
test/index.test.ts
Outdated
|
|
||
| requestWithAuth("GET /repos/octocat/hello-world").catch((error) => { | ||
| expect(error.message).toBe( | ||
| `[@octokit/auth-app] After 3 retries within 6s after creating the installation access token, the response remains 401. At this point it's unlikely a replication lag, but a legit authentication problem or a system outage, please check https://www.githubstatus.com/` |
There was a problem hiding this comment.
Thanks for the ping 👍 Additionally, are there any error messages returned by the API with the 401 that would make sense to surface in Octokit's error message?
test/index.test.ts
Outdated
|
|
||
| requestWithAuth("GET /repos/octocat/hello-world").catch((error) => { | ||
| expect(error.message).toBe( | ||
| `[@octokit/auth-app] After 3 retries within 6s after creating the installation access token, the response remains 401. At this point it's unlikely a replication lag, but a legit authentication problem or a system outage, please check https://www.githubstatus.com/` |
There was a problem hiding this comment.
Formatting suggestion:
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.
| error.message = `[@octokit/auth-app] After ${retries} retries within ${ | ||
| timeSinceTokenCreationInMs / 1000 | ||
| }s after creating the installation access token, the response remains 401. At this point it's unlikely a replication lag, but a legit authentication problem or a system outage, please check https://www.githubstatus.com/`; | ||
| } |
There was a problem hiding this comment.
The comment on line 52 should be updated to 5 seconds instead of 1 minute.
I like this better, thanks @imwiss. Resolves https://github.com/octokit/auth-app.js/pull/168/files\#r486622394
|
🎉 This PR is included in version 2.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #163