fix(requests): got calls will be retried on response code 429 #361 #728
Conversation
JonasSchubert
force-pushed
the
feat/use-got-retry-if-too-many-requests-error-is-returned-#361
branch
from
aaf1091 to
e8ff582
Compare
…c-release#361 If got receives a response code 429 - too many requests - it will retry up to three times for GET and POST requests. This uses the retry API https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md
JonasSchubert
force-pushed
the
feat/use-got-retry-if-too-many-requests-error-is-returned-#361
branch
from
3a0ec19 to
333a8ea
Compare
|
Might also help to fix #529 |
|
|
||
| export const RETRY_CONFIGURATION = { | ||
| retry: { | ||
| limit: 3, |
There was a problem hiding this comment.
Default in got is 2. I am open for discussion.
| @@ -1,3 +1,10 @@ | |||
| export const HOME_URL = 'https://github.com/semantic-release/semantic-release'; | |||
|
|
|||
| export const RELEASE_NAME = 'GitLab release'; | |||
|
|
|||
| export const RETRY_CONFIGURATION = { | |||
| retry: { | |||
There was a problem hiding this comment.
We could try to calculate the retryAfter property into optimizing the calculation for the next try.
There was a problem hiding this comment.
I think got does that already, see https://github.com/sindresorhus/got/blob/cc346b79561b13e3a266b22b7f612fca5a45eb1e/source/core/index.ts#L357
There was a problem hiding this comment.
Yes indeed. I assumed we are overriding the retry options when setting the apiOptions. I was wrong. Thanks for properly looking into it. I will close this PR.
What is still confusing though are the reported 429 responses of some users...
| @@ -567,7 +567,7 @@ test.serial("Throw error if GitLab API return any other errors", async (t) => { | |||
| const owner = "test_user"; | |||
| const repo = "test_repo"; | |||
| const env = { GITLAB_TOKEN: "gitlab_token" }; | |||
| const gitlab = authenticate(env).get(`/projects/${owner}%2F${repo}`).times(3).reply(500); | |||
| const gitlab = authenticate(env).get(`/projects/${owner}%2F${repo}`).times(3).reply(500).persist(); | |||
There was a problem hiding this comment.
.persist is needed to avoid confusing nock errors.
| export const RETRY_CONFIGURATION = { | ||
| retry: { | ||
| limit: 3, | ||
| statusCodes: [429], |
There was a problem hiding this comment.
I think we can go with the default here:
| statusCodes: [429], |
There was a problem hiding this comment.
I will close this PR. Thanks for reviewing!
|
Thanks for the PR @JonasSchubert! I was not aware that Have you specifically noticed issues with the default configuration (2 retries, more status codes)? |
| @@ -1,3 +1,10 @@ | |||
| export const HOME_URL = 'https://github.com/semantic-release/semantic-release'; | |||
|
|
|||
| export const RELEASE_NAME = 'GitLab release'; | |||
|
|
|||
| export const RETRY_CONFIGURATION = { | |||
| retry: { | |||
There was a problem hiding this comment.
Yes indeed. I assumed we are overriding the retry options when setting the apiOptions. I was wrong. Thanks for properly looking into it. I will close this PR.
What is still confusing though are the reported 429 responses of some users...
| export const RETRY_CONFIGURATION = { | ||
| retry: { | ||
| limit: 3, | ||
| statusCodes: [429], |
There was a problem hiding this comment.
I will close this PR. Thanks for reviewing!
We still raise the error after retrying after 2 retries, and also https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md#maxretryafter might be relevant here. |
If got receives a response code 429 - too many requests - it will retry up to three times.
This utilizes the retry API https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md
I am open to change the amount of retries as I selected three as a - for me - good first choice (third time's a charm 😄)