Skip to content

Commit

Permalink
Stop automatic retrying on RateLimitExceeded (#210)
Browse files Browse the repository at this point in the history
It does not make sense to retry when the rate limit is already exceeded. However, it makes sense to retry on Conflict. In the best case this would happen on the Caller/User side, but removing the retry completely would be a breaking change.

Signed-off-by: Lukas Kämmerling <lukas.kaemmerling@hetzner-cloud.de>
  • Loading branch information
LKaemmerling committed Sep 19, 2022
1 parent 3446580 commit ba4bea1
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 15 deletions.
6 changes: 3 additions & 3 deletions hcloud/action_test.go
Expand Up @@ -183,11 +183,11 @@ func TestActionClientWatchProgress(t *testing.T) {
},
})
case 2:
w.WriteHeader(http.StatusTooManyRequests)
w.WriteHeader(http.StatusConflict)
_ = json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeRateLimitExceeded),
Message: "ratelimited",
Code: string(ErrorCodeConflict),
Message: "conflict",
},
})
return
Expand Down
6 changes: 3 additions & 3 deletions hcloud/client.go
Expand Up @@ -270,7 +270,7 @@ func (c *Client) Do(r *http.Request, v interface{}) (*Response, error) {
err = errorFromResponse(resp, body)
if err == nil {
err = fmt.Errorf("hcloud: server responded with status code %d", resp.StatusCode)
} else if isRetryable(err) {
} else if isConflict(err) {
c.backoff(retries)
retries++
continue
Expand All @@ -289,12 +289,12 @@ func (c *Client) Do(r *http.Request, v interface{}) (*Response, error) {
}
}

func isRetryable(error error) bool {
func isConflict(error error) bool {
err, ok := error.(Error)
if !ok {
return false
}
return err.Code == ErrorCodeRateLimitExceeded || err.Code == ErrorCodeConflict
return err.Code == ErrorCodeConflict
}

func (c *Client) backoff(retries int) {
Expand Down
18 changes: 9 additions & 9 deletions hcloud/client_test.go
Expand Up @@ -183,7 +183,7 @@ func TestClientAll(t *testing.T) {

var (
ctx = context.Background()
ratelimited bool
conflicting bool
expectedPage = 1
)

Expand All @@ -205,13 +205,13 @@ func TestClientAll(t *testing.T) {
respBody.Meta.Pagination.Page = 1
respBody.Meta.Pagination.NextPage = 2
case "2":
if !ratelimited {
ratelimited = true
w.WriteHeader(http.StatusTooManyRequests)
if !conflicting {
conflicting = true
w.WriteHeader(http.StatusConflict)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeRateLimitExceeded),
Message: "ratelimited",
Code: string(ErrorCodeConflict),
Message: "conflict",
},
})
return
Expand Down Expand Up @@ -262,11 +262,11 @@ func TestClientDo(t *testing.T) {
w.Header().Set("Content-Type", "application/json")
switch callCount {
case 1:
w.WriteHeader(http.StatusTooManyRequests)
w.WriteHeader(http.StatusConflict)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeRateLimitExceeded),
Message: "ratelimited",
Code: string(ErrorCodeConflict),
Message: "conflict",
},
})
case 2:
Expand Down

0 comments on commit ba4bea1

Please sign in to comment.