Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support errors.Is for returned errors. #114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
9 changes: 4 additions & 5 deletions httpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"time"

"github.com/gojek/heimdall/v7"
"github.com/gojek/valkyrie"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -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++ {
Expand All @@ -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)
Expand All @@ -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) {
Expand Down
35 changes: 35 additions & 0 deletions httpclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
45 changes: 45 additions & 0 deletions httpclient/multiError.go
Original file line number Diff line number Diff line change
@@ -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()
}