diff --git a/Makefile b/Makefile index a8552e64..9b005bfa 100644 --- a/Makefile +++ b/Makefile @@ -3,3 +3,6 @@ TEST?=$$(go list ./... | grep -v 'vendor') test: @go test $(TEST) || exit 1 @echo $(TEST) | xargs -t -n4 go test $(TESTARGS) -timeout=30s -parallel=4 + +quality: + @goreportcard-cli -v ./... diff --git a/client.go b/client.go index 82c46520..43962c43 100644 --- a/client.go +++ b/client.go @@ -25,6 +25,7 @@ import ( "github.com/SchwarzIT/community-stackit-go-client/pkg/api/v2/membership" resourceManager "github.com/SchwarzIT/community-stackit-go-client/pkg/api/v2/resource-manager" "github.com/SchwarzIT/community-stackit-go-client/pkg/consts" + "github.com/SchwarzIT/community-stackit-go-client/pkg/retry" "github.com/SchwarzIT/community-stackit-go-client/pkg/validate" "golang.org/x/oauth2" ) @@ -33,6 +34,7 @@ import ( type Client struct { client *http.Client config *Config + retry *retry.Retry // Productive services - services that are ready to be used in production ProductiveServices @@ -52,6 +54,14 @@ func New(ctx context.Context, cfg *Config) (*Client, error) { return c.init(ctx), nil } +// WithRetry sets retry.Retry in a shallow copy of the given client +// and returns the new copy +func (c *Client) WithRetry(r *retry.Retry) *Client { + nc := *c + nc.retry = r + return &nc +} + // Service management // ProductiveServices is the struct representing all productive services @@ -137,8 +147,23 @@ func (c *Client) Request(ctx context.Context, method, path string, body []byte) return req, nil } -// Do performs the request and decodes the response if given interface != nil +// Do performs the request, including retry if set +// To set retry, use WithRetry() which returns a shalow copy of the client func (c *Client) Do(req *http.Request, v interface{}, errorHandlers ...func(*http.Response) error) (*http.Response, error) { + if c.retry == nil { + return c.do(req, v, errorHandlers...) + } + return c.doWithRetry(req, v, errorHandlers...) +} + +func (c *Client) doWithRetry(req *http.Request, v interface{}, errorHandlers ...func(*http.Response) error) (*http.Response, error) { + return c.retry.Do(req, func(r *http.Request) (*http.Response, error) { + return c.do(r, v, errorHandlers...) + }) +} + +// Do performs the request and decodes the response if given interface != nil +func (c *Client) do(req *http.Request, v interface{}, errorHandlers ...func(*http.Response) error) (*http.Response, error) { resp, err := c.client.Do(req) if err != nil { return nil, err diff --git a/client_test.go b/client_test.go index 5c3f59b8..e02f19b0 100644 --- a/client_test.go +++ b/client_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/SchwarzIT/community-stackit-go-client/pkg/consts" + "github.com/SchwarzIT/community-stackit-go-client/pkg/retry" ) func TestNew(t *testing.T) { @@ -274,3 +275,21 @@ func TestClient_SetToken(t *testing.T) { }) } } + +func TestClient_DoWithRetryNonRetryableError(t *testing.T) { + c, mux, teardown, err := MockServer() + defer teardown() + if err != nil { + t.Errorf("error from mock.AuthServer: %s", err.Error()) + } + + mux.HandleFunc("/err", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + }) + + req, _ := c.Request(context.Background(), http.MethodGet, "/err", nil) + if _, err := c.WithRetry(retry.New()).Do(req, nil); err == nil { + t.Error("expected do request to return error but got nil instead") + } +} diff --git a/pkg/retry/is_retryable.go b/pkg/retry/is_retryable.go new file mode 100644 index 00000000..dbe4a1bc --- /dev/null +++ b/pkg/retry/is_retryable.go @@ -0,0 +1,54 @@ +package retry + +import ( + "net/http" + "strings" +) + +const ( + // timeout configuration constants + CONFIG_IS_RETRYABLE = "IsRetryable" + + // requesrt errors + ERR_NO_SUCH_HOST = "dial tcp: lookup" +) + +// IsRetryable is the config struct +type IsRetryable struct { + fnList []func(err error) bool +} + +// SetIsRetryable sets functions that determin if an error can be retried or not +func (c *Retry) SetIsRetryable(f ...func(err error) bool) *Retry { + return c.withConfig(&IsRetryable{ + fnList: f, + }) +} + +var _ = Config(&IsRetryable{}) + +func (c *IsRetryable) String() string { + return CONFIG_IS_RETRYABLE +} + +func (c *IsRetryable) Value() interface{} { + return c.fnList +} + +// IsRetryableNoOp always retries +func IsRetryableNoOp(err error) bool { + return true +} + +// IsRetryableDefault +func IsRetryableDefault(err error) bool { + if strings.Contains(err.Error(), http.StatusText(http.StatusBadRequest)) { + return strings.Contains(err.Error(), ERR_NO_SUCH_HOST) + } + + if strings.Contains(err.Error(), http.StatusText(http.StatusUnauthorized)) { + return false + } + + return true +} diff --git a/pkg/retry/is_retryable_test.go b/pkg/retry/is_retryable_test.go new file mode 100644 index 00000000..d910a4d4 --- /dev/null +++ b/pkg/retry/is_retryable_test.go @@ -0,0 +1,88 @@ +package retry + +import ( + "errors" + "net/http" + "reflect" + "runtime" + "testing" +) + +func GetFunctionName(i interface{}) string { + return runtime.FuncForPC(reflect.ValueOf(i).Pointer()).Name() +} + +func TestRetry_SetIsRetryable(t *testing.T) { + r := New() + r.IsRetryableFns = []func(err error) bool{IsRetryableNoOp} + + type args struct { + f []func(err error) bool + } + tests := []struct { + name string + args args + want *Retry + }{ + {"ok", args{[]func(err error) bool{IsRetryableNoOp}}, r}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := New() + got := c.SetIsRetryable(tt.args.f...) + if len(got.IsRetryableFns) != len(r.IsRetryableFns) { + t.Error("wrong lengths") + return + } + for k, v := range got.IsRetryableFns { + if GetFunctionName(r.IsRetryableFns[k]) != GetFunctionName(v) { + t.Errorf("%s != %s", GetFunctionName(r.IsRetryableFns[k]), GetFunctionName(v)) + return + } + } + }) + } +} + +func TestIsRetryableNoOp(t *testing.T) { + type args struct { + err error + } + tests := []struct { + name string + args args + want bool + }{ + {"always true", args{nil}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsRetryableNoOp(tt.args.err); got != tt.want { + t.Errorf("IsRetryableNoOp() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestIsRetryableDefault(t *testing.T) { + type args struct { + err error + } + tests := []struct { + name string + args args + want bool + }{ + {"bad request - no retry", args{errors.New(http.StatusText(http.StatusBadRequest))}, false}, + {"bad request - no host - retry", args{errors.New(http.StatusText(http.StatusBadRequest) + ERR_NO_SUCH_HOST)}, true}, + {"bad request - unauthorized", args{errors.New(http.StatusText(http.StatusUnauthorized))}, false}, + {"other error", args{errors.New(http.StatusText(http.StatusConflict))}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsRetryableDefault(tt.args.err); got != tt.want { + t.Errorf("IsRetryableDefault() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/retry/max_retries.go b/pkg/retry/max_retries.go new file mode 100644 index 00000000..7203d43a --- /dev/null +++ b/pkg/retry/max_retries.go @@ -0,0 +1,30 @@ +package retry + +const ( + // name of the configuration + CONFIG_MAX_RETRIES = "MaxRetries" +) + +// MaxRetries is the config struct +type MaxRetries struct { + Retries *int +} + +// SetMaxRetries sets the maximum retries +func (c *Retry) SetMaxRetries(r int) *Retry { + return c.withConfig(&MaxRetries{ + Retries: &r, + }) +} + +var _ = Config(&Timeout{}) + +// String returns the config name +func (c *MaxRetries) String() string { + return CONFIG_MAX_RETRIES +} + +// Value return the value +func (c *MaxRetries) Value() interface{} { + return c.Retries +} diff --git a/pkg/retry/max_retries_test.go b/pkg/retry/max_retries_test.go new file mode 100644 index 00000000..826a9b89 --- /dev/null +++ b/pkg/retry/max_retries_test.go @@ -0,0 +1,30 @@ +package retry + +import ( + "reflect" + "testing" +) + +func TestRetry_SetMaxRetries(t *testing.T) { + r := New() + five := 5 + r.MaxRetries = &five + type args struct { + r int + } + tests := []struct { + name string + args args + want *Retry + }{ + {"ok", args{5}, r}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := New() + if got := c.SetMaxRetries(tt.args.r); !reflect.DeepEqual(*got.MaxRetries, *tt.want.MaxRetries) { + t.Errorf("Retry.SetMaxRetries() = %v, want %v", *got.MaxRetries, *tt.want.MaxRetries) + } + }) + } +} diff --git a/pkg/retry/retry.go b/pkg/retry/retry.go new file mode 100644 index 00000000..6d997650 --- /dev/null +++ b/pkg/retry/retry.go @@ -0,0 +1,115 @@ +package retry + +import ( + "context" + "net/http" + "time" + + "github.com/pkg/errors" +) + +type Retry struct { + Timeout time.Duration // max duration + MaxRetries *int // max retries, when nil, there's no retry limit + Throttle time.Duration // wait duration between calls + IsRetryableFns []func(err error) bool // functions to determine if error is retriable +} + +type Config interface { + String() string + Value() interface{} +} + +// New returns a new instance of Retry with default values +func New() *Retry { + c := &Retry{ + Timeout: CONFIG_TIMEOUT_DEFAULT, + MaxRetries: nil, + Throttle: CONFIG_THROTTLE_DEFAULT, + IsRetryableFns: []func(err error) bool{IsRetryableDefault}, + } + return c +} + +func (c *Retry) withConfig(cfgs ...Config) *Retry { + for _, cfg := range cfgs { + switch cfg.String() { + case CONFIG_TIMEOUT: + c.Timeout = cfg.Value().(time.Duration) + case CONFIG_THROTTLE: + c.Throttle = cfg.Value().(time.Duration) + case CONFIG_MAX_RETRIES: + c.MaxRetries = cfg.Value().(*int) + case CONFIG_IS_RETRYABLE: + c.IsRetryableFns = cfg.Value().([]func(err error) bool) + } + } + return c +} + +// Do runs a given do function with a given request -> do(req) +func (r *Retry) Do(req *http.Request, do func(*http.Request) (*http.Response, error)) (*http.Response, error) { + var lastErr error + var res *http.Response + var retry bool + + overall, cancel := context.WithTimeout(req.Context(), r.Timeout) + defer cancel() + + // set overall ctx in request + newReq := req.WithContext(overall) + + // clone max retries + maxRetries := -1 + if r.MaxRetries != nil { + maxRetries = *r.MaxRetries + } + + for { + res, maxRetries, retry, lastErr = r.doIteration(newReq, do, maxRetries) + if lastErr == nil || !retry { + return res, lastErr + } + + // context timeout was chosen in order to support throttle = 0 + tick, cancelTick := context.WithTimeout(context.Background(), r.Throttle) + defer cancelTick() + + select { + case <-tick.Done(): + // continue + case <-overall.Done(): + return nil, errors.Wrap(lastErr, "retry context timed out") + } + } +} + +func (r *Retry) doIteration(req *http.Request, do func(*http.Request) (*http.Response, error), retries int) (res *http.Response, retriesLeft int, retry bool, err error) { + retriesLeft = retries + retry = true + + res, err = do(req) + if err == nil { + retry = false + return + } + + // check if error is retryable + for _, f := range r.IsRetryableFns { + if !f(err) { + retry = false + return + } + } + + if retries != -1 { + if retries == 0 { + err = errors.Wrap(err, "reached max retries") + retry = false + return + } + retriesLeft = retries - 1 + } + + return +} diff --git a/pkg/retry/retry_test.go b/pkg/retry/retry_test.go new file mode 100644 index 00000000..54f42560 --- /dev/null +++ b/pkg/retry/retry_test.go @@ -0,0 +1,138 @@ +package retry_test + +import ( + "context" + "fmt" + "net/http" + "strings" + "testing" + "time" + + client "github.com/SchwarzIT/community-stackit-go-client" + "github.com/SchwarzIT/community-stackit-go-client/pkg/retry" +) + +func TestNew(t *testing.T) { + tests := []struct { + name string + want *retry.Retry + }{ + {"ok", retry.New()}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := retry.New() + if got.MaxRetries != tt.want.MaxRetries || + got.Throttle != tt.want.Throttle || + got.Timeout != tt.want.Timeout || + len(got.IsRetryableFns) != len(tt.want.IsRetryableFns) { + t.Error("one or more values don't match") + } + }) + } +} + +const ( + response_before_2s = `{"status":false}` + response_after_2s = `{"status":true}` +) + +func TestClient_DoWithRetryThrottle(t *testing.T) { + c, mux, teardown, err := client.MockServer() + defer teardown() + if err != nil { + t.Errorf("error from mock.AuthServer: %s", err.Error()) + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) + defer cancel() + mux.HandleFunc("/2s", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if ctx.Err() != nil { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, response_after_2s) + } else { + w.WriteHeader(http.StatusLocked) + fmt.Fprint(w, response_before_2s) + } + }) + + c = c.WithRetry(retry.New().SetThrottle(1 * time.Second)) + + req, _ := c.Request(context.Background(), http.MethodGet, "/2s", nil) + + var got struct { + Status bool `json:"status"` + } + + if _, err := c.Do(req, &got); err != nil { + t.Errorf("do request: %v", err) + } + + if !got.Status { + t.Errorf("received status = %v", got.Status) + } +} + +func TestClient_DoWithRetryNonRetryableError(t *testing.T) { + c, mux, teardown, err := client.MockServer() + defer teardown() + if err != nil { + t.Errorf("error from mock.AuthServer: %s", err.Error()) + } + + mux.HandleFunc("/err", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + }) + + c = c.WithRetry(retry.New()) + req, _ := c.Request(context.Background(), http.MethodGet, "/err", nil) + if _, err := c.Do(req, nil); err == nil { + t.Error("expected do request to return error but got nil instead") + } +} + +func TestClient_DoWithRetryMaxRetries(t *testing.T) { + c, mux, teardown, err := client.MockServer() + defer teardown() + if err != nil { + t.Errorf("error from mock.AuthServer: %s", err.Error()) + } + + mux.HandleFunc("/err", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusLocked) + }) + + c = c.WithRetry(retry.New().SetThrottle(1 * time.Second).SetMaxRetries(2)) + req, _ := c.Request(context.Background(), http.MethodGet, "/err", nil) + if _, err := c.Do(req, nil); !strings.Contains(err.Error(), "reached max retries") { + t.Errorf("expected do request to return max retries error but got %q instead", err) + } +} + +func TestClient_DoWithRetryTimeout(t *testing.T) { + c, mux, teardown, err := client.MockServer() + defer teardown() + if err != nil { + t.Errorf("error from mock.AuthServer: %s", err.Error()) + } + + mux.HandleFunc("/err", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusLocked) + }) + + c = c.WithRetry(retry.New().SetTimeout(5 * time.Second)) + req, _ := c.Request(context.Background(), http.MethodGet, "/err", nil) + if _, err := c.Do(req, nil); !strings.Contains(err.Error(), "retry context timed out") && !strings.Contains(err.Error(), http.StatusText(http.StatusLocked)) { + t.Errorf("expected do request to return retry context timed out error with locked error status but got '%v' instead", err) + } + + c = c.WithRetry(retry.New().SetTimeout(0 * time.Second)) + req, _ = c.Request(context.Background(), http.MethodGet, "/err", nil) + if _, err := c.Do(req, nil); !strings.Contains(err.Error(), "retry context timed out") { + t.Errorf("expected do request to return retry context timed out error but got '%v' instead", err) + } +} diff --git a/pkg/retry/throttle.go b/pkg/retry/throttle.go new file mode 100644 index 00000000..d893f8af --- /dev/null +++ b/pkg/retry/throttle.go @@ -0,0 +1,33 @@ +package retry + +import "time" + +const ( + // timeout configuration constants + CONFIG_THROTTLE = "Throttle" + CONFIG_THROTTLE_DEFAULT = 5 * time.Second +) + +// Throttle is the config struct +type Throttle struct { + Duration time.Duration +} + +// SetThrottle sets the duration to wait between calls +func (c *Retry) SetThrottle(d time.Duration) *Retry { + return c.withConfig(&Throttle{ + Duration: d, + }) +} + +var _ = Config(&Throttle{}) + +// String return the name of the config +func (c *Throttle) String() string { + return CONFIG_THROTTLE +} + +// Value returns the defined value +func (c *Throttle) Value() interface{} { + return c.Duration +} diff --git a/pkg/retry/throttle_test.go b/pkg/retry/throttle_test.go new file mode 100644 index 00000000..8756674a --- /dev/null +++ b/pkg/retry/throttle_test.go @@ -0,0 +1,30 @@ +package retry + +import ( + "reflect" + "testing" + "time" +) + +func TestRetry_SetThrottle(t *testing.T) { + r := New() + r.Throttle = time.Minute + type args struct { + d time.Duration + } + tests := []struct { + name string + args args + want *Retry + }{ + {"ok", args{time.Minute}, r}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := New() + if got := c.SetThrottle(tt.args.d); !reflect.DeepEqual(got.Throttle, tt.want.Throttle) { + t.Errorf("Retry.SetThrottle() = %v, want %v", got.Throttle, tt.want.Throttle) + } + }) + } +} diff --git a/pkg/retry/timeout.go b/pkg/retry/timeout.go new file mode 100644 index 00000000..d2f801ed --- /dev/null +++ b/pkg/retry/timeout.go @@ -0,0 +1,33 @@ +package retry + +import "time" + +const ( + // timeout configuration constants + CONFIG_TIMEOUT = "Timeout" + CONFIG_TIMEOUT_DEFAULT = 60 * time.Minute +) + +// Timeout is the config struct +type Timeout struct { + Duration time.Duration +} + +// SetTimeout sets the maximum run duration +func (c *Retry) SetTimeout(d time.Duration) *Retry { + return c.withConfig(&Timeout{ + Duration: d, + }) +} + +var _ = Config(&Timeout{}) + +// String return the name of the config +func (c *Timeout) String() string { + return CONFIG_TIMEOUT +} + +// Value returns the defined value +func (c *Timeout) Value() interface{} { + return c.Duration +} diff --git a/pkg/retry/timeout_test.go b/pkg/retry/timeout_test.go new file mode 100644 index 00000000..ca579b8e --- /dev/null +++ b/pkg/retry/timeout_test.go @@ -0,0 +1,30 @@ +package retry + +import ( + "reflect" + "testing" + "time" +) + +func TestRetry_SetTimeout(t *testing.T) { + r := New() + r.Timeout = 1 * time.Minute + type args struct { + d time.Duration + } + tests := []struct { + name string + args args + want *Retry + }{ + {"all ok", args{time.Minute}, r}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := New() + if got := c.SetTimeout(tt.args.d); !reflect.DeepEqual(got.Timeout, tt.want.Timeout) { + t.Errorf("Retry.SetTimeout() = %v, want %v", got.Timeout, tt.want.Timeout) + } + }) + } +}