From b80600e50d76450a7e6239427f6e9795de89db71 Mon Sep 17 00:00:00 2001 From: Peter Zeller Date: Tue, 13 Jul 2021 09:57:44 +0200 Subject: [PATCH] replace valkyrie.MultiError with custom MultiError - supports errors.Is and errors.Cause - improved performance (no mutex required) --- go.mod | 1 - go.sum | 3 --- httpclient/client.go | 9 ++++---- httpclient/client_test.go | 35 ++++++++++++++++++++++++++++++ httpclient/multiError.go | 45 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 httpclient/multiError.go diff --git a/go.mod b/go.mod index 57b691f..a3dbb7a 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( github.com/DataDog/datadog-go v3.7.1+incompatible // indirect github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5 github.com/cactus/go-statsd-client/statsd v0.0.0-20200423205355-cb0885a1018c // indirect - github.com/gojek/valkyrie v0.0.0-20180215180059-6aee720afcdf github.com/pkg/errors v0.9.1 github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect github.com/smartystreets/goconvey v1.6.4 // indirect diff --git a/go.sum b/go.sum index 953d375..ecb9d3d 100644 --- a/go.sum +++ b/go.sum @@ -4,12 +4,9 @@ github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5 h1:rFw4nCn9iMW+Vaj github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c= github.com/cactus/go-statsd-client/statsd v0.0.0-20200423205355-cb0885a1018c h1:HIGF0r/56+7fuIZw2V4isE22MK6xpxWx7BbV8dJ290w= github.com/cactus/go-statsd-client/statsd v0.0.0-20200423205355-cb0885a1018c/go.mod h1:l/bIBLeOl9eX+wxJAzxS4TveKRtAqlyDpHjhkfO0MEI= -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/gojek/valkyrie v0.0.0-20180215180059-6aee720afcdf h1:5xRGbUdOmZKoDXkGx5evVLehuCMpuO1hl701bEQqXOM= -github.com/gojek/valkyrie v0.0.0-20180215180059-6aee720afcdf/go.mod h1:QzhUKaYKJmcbTnCYCAVQrroCOY7vOOI8cSQ4NbuhYf0= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 h1:EGx4pi6eqNxGaHF6qqu48+N2wcFQ5qg5FXgOdqsJ5d8= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo= diff --git a/httpclient/client.go b/httpclient/client.go index 6880776..8aa73de 100644 --- a/httpclient/client.go +++ b/httpclient/client.go @@ -8,7 +8,6 @@ import ( "time" "github.com/gojek/heimdall/v7" - "github.com/gojek/valkyrie" "github.com/pkg/errors" ) @@ -135,7 +134,7 @@ func (c *Client) Do(request *http.Request) (*http.Response, error) { request.Body = ioutil.NopCloser(bodyReader) // prevents closing the body between retries } - multiErr := &valkyrie.MultiError{} + var errs []error var response *http.Response for i := 0; i <= c.retryCount; i++ { @@ -153,7 +152,7 @@ func (c *Client) Do(request *http.Request) (*http.Response, error) { } if err != nil { - multiErr.Push(err.Error()) + errs = append(errs, err) c.reportError(request, err) backoffTime := c.retrier.NextInterval(i) time.Sleep(backoffTime) @@ -167,11 +166,11 @@ func (c *Client) Do(request *http.Request) (*http.Response, error) { continue } - multiErr = &valkyrie.MultiError{} // Clear errors if any iteration succeeds + errs = nil // Clear errors if any iteration succeeds break } - return response, multiErr.HasError() + return response, MultiError{errs}.HasError() } func (c *Client) reportRequestStart(request *http.Request) { diff --git a/httpclient/client_test.go b/httpclient/client_test.go index 83e94ba..cf8f73b 100644 --- a/httpclient/client_test.go +++ b/httpclient/client_test.go @@ -2,14 +2,17 @@ package httpclient import ( "bytes" + "context" "io/ioutil" "net/http" "net/http/httptest" + "net/url" "strings" "testing" "time" "github.com/gojek/heimdall/v7" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -507,6 +510,38 @@ func TestCustomHTTPClientHeaderSuccess(t *testing.T) { assert.Equal(t, "{ \"response\": \"ok\" }", string(body)) } +func TestHTTPClientContextTimeout(t *testing.T) { + client := NewClient(WithHTTPTimeout(1000 * time.Millisecond)) + + dummyHandler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + assert.Equal(t, "en", r.Header.Get("Accept-Language")) + + time.Sleep(100 * time.Millisecond) + + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{ "response": "ok" }`)) + } + + server := httptest.NewServer(http.HandlerFunc(dummyHandler)) + defer server.Close() + + ctxt, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) + defer cancel() + + req, err := http.NewRequestWithContext(ctxt, http.MethodGet, server.URL, nil) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept-Language", "en") + response, err := client.Do(req) + require.Error(t, err, "should have timed out in GET request") + require.Contains(t, err.Error(), "context deadline exceeded") + require.True(t, errors.Is(err, context.DeadlineExceeded)) + assert.Equal(t, &url.Error{Op: "Get", URL: server.URL, Err: context.DeadlineExceeded}, errors.Cause(err)) + require.Nil(t, response) +} + func respBody(t *testing.T, response *http.Response) string { if response.Body != nil { defer response.Body.Close() diff --git a/httpclient/multiError.go b/httpclient/multiError.go new file mode 100644 index 0000000..560b90d --- /dev/null +++ b/httpclient/multiError.go @@ -0,0 +1,45 @@ +package httpclient + +import "strings" + +// MultiError is a container for a list of errors. +type MultiError struct { + errors []error +} + +// HasError checks if MultiError has any error. +func (m MultiError) HasError() error { + if len(m.errors) > 0 { + return m + } + return nil +} + +// MultiError implements error interface. +func (m MultiError) Error() string { + formattedError := make([]string, len(m.errors)) + + for i, err := range m.errors { + formattedError[i] = err.Error() + } + + return strings.Join(formattedError, ", ") +} + +// ErrorList returns a list of all errors in the MultiError with the first one at the front. +func (m MultiError) ErrorList() []error { + return m.errors +} + +// Cause returns the first error in the MultiError or nil if there are none +func (m MultiError) Cause() error { + if len(m.errors) == 0 { + return nil + } + return m.errors[0] +} + +// Unwrap returns the first error in the MultiError or nil if there are none +func (m MultiError) Unwrap() error { + return m.Cause() +}