diff --git a/hcloud/action_waiter_test.go b/hcloud/action_waiter_test.go index 0e7daf90d..ecea3a6d7 100644 --- a/hcloud/action_waiter_test.go +++ b/hcloud/action_waiter_test.go @@ -81,17 +81,39 @@ func TestWaitFor(t *testing.T) { }, }, { - Name: "fail with api error", + Name: "fail with server error", WantRequests: []mockutils.Request{ {Method: "GET", Path: "/actions?id=1509772237&page=1&sort=status&sort=id", - Status: 503}, + Status: 500}, }, Run: func(env testEnv) { actions := []*Action{{ID: 1509772237, Status: ActionStatusRunning}} err := env.Client.Action.WaitFor(context.Background(), actions...) assert.Error(t, err) - assert.Equal(t, "hcloud: server responded with status code 503", err.Error()) + assert.Equal(t, "hcloud: server responded with status code 500", err.Error()) + }, + }, + { + Name: "succeed with retry", + WantRequests: []mockutils.Request{ + {Method: "GET", Path: "/actions?id=1509772237&page=1&sort=status&sort=id", + Status: 503}, + {Method: "GET", Path: "/actions?id=1509772237&page=1&sort=status&sort=id", + Status: 200, + JSONRaw: `{ + "actions": [ + { "id": 1509772237, "status": "success", "progress": 100 } + ], + "meta": { "pagination": { "page": 1 }} + }`, + }, + }, + Run: func(env testEnv) { + actions := []*Action{{ID: 1509772237, Status: ActionStatusRunning}} + + err := env.Client.Action.WaitFor(context.Background(), actions...) + assert.NoError(t, err) }, }, }, diff --git a/hcloud/client_handler_error.go b/hcloud/client_handler_error.go index 23b326e31..0ba4fd2cb 100644 --- a/hcloud/client_handler_error.go +++ b/hcloud/client_handler_error.go @@ -2,12 +2,15 @@ package hcloud import ( "encoding/json" + "errors" "fmt" "net/http" "github.com/hetznercloud/hcloud-go/v2/hcloud/schema" ) +var ErrorStatusCode = errors.New("server responded with status code") + func wrapErrorHandler(wrapped handler) handler { return &errorHandler{wrapped} } @@ -25,7 +28,7 @@ func (h *errorHandler) Do(req *http.Request, v any) (resp *Response, err error) if resp.StatusCode >= 400 && resp.StatusCode <= 599 { err = errorFromBody(resp) if err == nil { - err = fmt.Errorf("hcloud: server responded with status code %d", resp.StatusCode) + err = fmt.Errorf("hcloud: %w %d", ErrorStatusCode, resp.StatusCode) } } return resp, err diff --git a/hcloud/client_handler_retry.go b/hcloud/client_handler_retry.go index 52347eb61..ec376b71e 100644 --- a/hcloud/client_handler_retry.go +++ b/hcloud/client_handler_retry.go @@ -1,6 +1,7 @@ package hcloud import ( + "errors" "net/http" "time" ) @@ -16,10 +17,11 @@ type retryHandler struct { func (h *retryHandler) Do(req *http.Request, v any) (resp *Response, err error) { retries := 0 + ctx := req.Context() for { // Clone the request using the original context - cloned, err := cloneRequest(req, req.Context()) + cloned, err := cloneRequest(req, ctx) if err != nil { return nil, err } @@ -30,13 +32,54 @@ func (h *retryHandler) Do(req *http.Request, v any) (resp *Response, err error) // - request preparation // - network connectivity // - http status code (see [errorHandler]) - if IsError(err, ErrorCodeConflict) { - time.Sleep(h.backoffFunc(retries)) - retries++ - continue + if ctx.Err() != nil { + return resp, ctx.Err() + } + + if retryPolicy(resp, err) { + select { + case <-ctx.Done(): + return resp, err + case <-time.After(h.backoffFunc(retries)): + retries++ + continue + } } } return resp, err } } + +func retryPolicy(resp *Response, err error) bool { + if err != nil { + var apiErr Error + + switch { + case errors.As(err, &apiErr): + if apiErr.Code == ErrorCodeConflict { + return true + } + case errors.Is(err, ErrorStatusCode): + if resp == nil || resp.Response == nil { + // Should not happen + return false + } + + if resp.Response.Request.Method != "GET" { + return false + } + + switch resp.Response.StatusCode { + // 4xx errors + case http.StatusTooManyRequests: + return true + // 5xx errors + case http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + return true + } + } + } + + return false +} diff --git a/hcloud/client_handler_retry_test.go b/hcloud/client_handler_retry_test.go index d575c7855..6ebda6196 100644 --- a/hcloud/client_handler_retry_test.go +++ b/hcloud/client_handler_retry_test.go @@ -28,14 +28,18 @@ func TestRetryHandler(t *testing.T) { }, { name: "http 503 error", - wrapped: func(_ *http.Request, _ any) (*Response, error) { - return nil, nil + wrapped: func(req *http.Request, _ any) (*Response, error) { + resp := fakeResponse(t, 503, "", false) + resp.Response.Request = req + return resp, fmt.Errorf("%w %d", ErrorStatusCode, 503) }, - want: 0, + want: 1, }, { name: "api conflict error", - wrapped: func(_ *http.Request, _ any) (*Response, error) { + wrapped: func(req *http.Request, _ any) (*Response, error) { + resp := fakeResponse(t, 409, "", false) + resp.Response.Request = req return nil, ErrorFromSchema(schema.Error{Code: string(ErrorCodeConflict)}) }, want: 1,