Skip to content

Commit

Permalink
chore: do not retry on bare HTTP 429 or HTTP 503 (#486)
Browse files Browse the repository at this point in the history
Related to #470

This is only a `chore` to prevent confusion with the now updated
release-please commits in #470

We prefer not to retry requests:
- If the ingress returns a "bare" 429, as the backend was not able to
respond anyway.
- If the ingress returns a "bare" 503, to prevent possible self-made
DDoS, and preventing a disaster recovery.
  • Loading branch information
jooola authored Jul 19, 2024
1 parent 19da475 commit 6d4413b
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 18 deletions.
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: []mockutil.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

0 comments on commit 6d4413b

Please sign in to comment.