Skip to content

Commit

Permalink
Fix errors from original http.Client response being double-wrapped wh…
Browse files Browse the repository at this point in the history
…en using the RoundTripper (#95)

This causes *url.Errors coming from the standard http.Client to be passed through instead of double wrapped.
  • Loading branch information
andrewloux authored Jun 9, 2020
1 parent 4af2e4b commit 7cf40fa
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
11 changes: 10 additions & 1 deletion roundtripper.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package retryablehttp

import (
"errors"
"net/http"
"net/url"
"sync"
)

Expand Down Expand Up @@ -39,5 +41,12 @@ func (rt *RoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
}

// Execute the request.
return rt.Client.Do(retryableReq)
resp, err := rt.Client.Do(retryableReq)
// If we got an error returned by standard library's `Do` method, unwrap it
// otherwise we will wind up erroneously re-nesting the error.
if _, ok := err.(*url.Error); ok {
return resp, errors.Unwrap(err)
}

return resp, err
}
52 changes: 52 additions & 0 deletions roundtripper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ package retryablehttp

import (
"context"
"errors"
"io/ioutil"
"net"
"net/http"
"net/http/httptest"
"net/url"
"reflect"
"sync/atomic"
"testing"
)
Expand Down Expand Up @@ -87,3 +91,51 @@ func TestRoundTripper_RoundTrip(t *testing.T) {
t.Fatalf("expected %q, got %q", "success!", v)
}
}

func TestRoundTripper_TransportFailureErrorHandling(t *testing.T) {
// Make a client with some custom settings to verify they are used.
retryClient := NewClient()
retryClient.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) {
if err != nil {
return true, err
}

return false, nil
}

retryClient.ErrorHandler = PassthroughErrorHandler

expectedError := &url.Error{
Op: "Get",
URL: "http://this-url-does-not-exist-ed2fb.com/",
Err: &net.OpError{
Op: "dial",
Net: "tcp",
Err: &net.DNSError{
Name: "this-url-does-not-exist-ed2fb.com",
Err: "no such host",
IsNotFound: true,
},
},
}

// Get the standard client and execute the request.
client := retryClient.StandardClient()
_, err := client.Get("http://this-url-does-not-exist-ed2fb.com/")

// assert expectations
if !reflect.DeepEqual(normalizeError(err), expectedError) {
t.Fatalf("expected %q, got %q", expectedError, err)
}
}

func normalizeError(err error) error {
var dnsError *net.DNSError

if errors.As(err, &dnsError) {
// this field is populated with the DNS server on on CI, but not locally
dnsError.Server = ""
}

return err
}

0 comments on commit 7cf40fa

Please sign in to comment.