Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: do not retry on bare HTTP 429 or HTTP 503 #486

Merged
merged 1 commit into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hcloud/action_waiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func TestWaitFor(t *testing.T) {
Name: "succeed with retry",
WantRequests: []mockutils.Request{
{Method: "GET", Path: "/actions?id=1509772237&page=1&sort=status&sort=id",
Status: 503,
Status: 502,
},
{Method: "GET", Path: "/actions?id=1509772237&page=1&sort=status&sort=id",
Status: 200,
Expand Down
5 changes: 1 addition & 4 deletions hcloud/client_handler_retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,8 @@ func retryPolicy(resp *Response, err error) bool {
}
case errors.Is(err, ErrStatusCode):
switch resp.Response.StatusCode {
// 4xx errors
case http.StatusTooManyRequests:
return true
// 5xx errors
case http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout:
case http.StatusBadGateway, http.StatusGatewayTimeout:
return true
}
case errors.As(err, &netErr):
Expand Down
25 changes: 14 additions & 11 deletions hcloud/client_handler_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ func TestRetryHandler(t *testing.T) {
},
},
{
name: "http 503 error recovery",
name: "http 502 error recovery",
wrapped: func(req *http.Request, _ any) (*Response, error) {
resp := fakeResponse(t, 503, "", false)
resp := fakeResponse(t, 502, "", false)
resp.Response.Request = req
return resp, fmt.Errorf("%w %d", ErrStatusCode, 503)
return resp, fmt.Errorf("%w %d", ErrStatusCode, 502)
},
recover: true,
want: func(t *testing.T, err error, retryCount int) {
Expand All @@ -44,14 +44,14 @@ func TestRetryHandler(t *testing.T) {
},
},
{
name: "http 503 error",
name: "http 502 error",
wrapped: func(req *http.Request, _ any) (*Response, error) {
resp := fakeResponse(t, 503, "", false)
resp := fakeResponse(t, 502, "", false)
resp.Response.Request = req
return resp, fmt.Errorf("%w %d", ErrStatusCode, 503)
return resp, fmt.Errorf("%w %d", ErrStatusCode, 502)
},
want: func(t *testing.T, err error, retryCount int) {
assert.EqualError(t, err, "server responded with status code 503")
assert.EqualError(t, err, "server responded with status code 502")
assert.Equal(t, 5, retryCount)
},
},
Expand Down Expand Up @@ -116,16 +116,19 @@ func TestRetryPolicy(t *testing.T) {
want bool
}{
{
// The API error code unavailable is used in many unexpected situations, we must only
// retry if the server returns HTTP 503.
name: "server returns 502 error",
resp: fakeResponse(t, 502, ``, false),
want: true,
},
{
name: "api returns unavailable error",
resp: fakeResponse(t, 503, `{"error":{"code":"unavailable"}}`, true),
want: false,
},
{
name: "server returns 503 error",
resp: fakeResponse(t, 503, ``, false),
want: true,
want: false,
},
{
name: "api returns rate_limit_exceeded error",
Expand All @@ -135,7 +138,7 @@ func TestRetryPolicy(t *testing.T) {
{
name: "server returns 429 error",
resp: fakeResponse(t, 429, ``, false),
want: true,
want: false,
},
{
name: "api returns conflict error",
Expand Down
2 changes: 0 additions & 2 deletions hcloud/hcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ The following rules defines when a request can be retried:
When the [http.Client] returned a network timeout error.
When the API returned an HTTP error, with the status code:
- [http.StatusTooManyRequests]
- [http.StatusBadGateway]
- [http.StatusServiceUnavailable]
- [http.StatusGatewayTimeout]
When the API returned an application error, with the code:
Expand Down
Loading