Skip to content

Commit

Permalink
Align ErrorPropagatedRetryPolicy with DefaultRetryPolicy (hashicorp#116)
Browse files Browse the repository at this point in the history
* Realign behavior of ErrorPropagatedRetryPolicy with DefaultRetryPolicy

ErrorPropagatedRetryPolicy should have been modified in hashicorp#100,
but failed to... probably because it has been merged approximately at the same time
as hashicorp#70 (the PR adding ErrorPropagatedRetryPolicy).

* Remove code duplication between DefaultRetryPolicy and ErrorPropagatedRetryPolicy
  • Loading branch information
maximerety authored Nov 4, 2020
1 parent 6861976 commit 991b9d0
Showing 1 changed file with 14 additions and 38 deletions.
52 changes: 14 additions & 38 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,44 +404,9 @@ func DefaultRetryPolicy(ctx context.Context, resp *http.Response, err error) (bo
return false, ctx.Err()
}

if err != nil {
if v, ok := err.(*url.Error); ok {
// Don't retry if the error was due to too many redirects.
if redirectsErrorRe.MatchString(v.Error()) {
return false, nil
}

// Don't retry if the error was due to an invalid protocol scheme.
if schemeErrorRe.MatchString(v.Error()) {
return false, nil
}

// Don't retry if the error was due to TLS cert verification failure.
if _, ok := v.Err.(x509.UnknownAuthorityError); ok {
return false, nil
}
}

// The error is likely recoverable so retry.
return true, nil
}

// 429 Too Many Requests is recoverable. Sometimes the server puts
// a Retry-After response header to indicate when the server is
// available to start processing request from client.
if resp.StatusCode == http.StatusTooManyRequests {
return true, nil
}

// Check the response code. We retry on 500-range responses to allow
// the server time to recover, as 500's are typically not permanent
// errors and may relate to outages on the server side. This will catch
// invalid response codes as well, like 0 and 999.
if resp.StatusCode == 0 || (resp.StatusCode >= 500 && resp.StatusCode != 501) {
return true, nil
}

return false, nil
// don't propagate other errors
shouldRetry, _ := baseRetryPolicy(resp, err)
return shouldRetry, nil
}

// ErrorPropagatedRetryPolicy is the same as DefaultRetryPolicy, except it
Expand All @@ -453,6 +418,10 @@ func ErrorPropagatedRetryPolicy(ctx context.Context, resp *http.Response, err er
return false, ctx.Err()
}

return baseRetryPolicy(resp, err)
}

func baseRetryPolicy(resp *http.Response, err error) (bool, error) {
if err != nil {
if v, ok := err.(*url.Error); ok {
// Don't retry if the error was due to too many redirects.
Expand All @@ -475,6 +444,13 @@ func ErrorPropagatedRetryPolicy(ctx context.Context, resp *http.Response, err er
return true, nil
}

// 429 Too Many Requests is recoverable. Sometimes the server puts
// a Retry-After response header to indicate when the server is
// available to start processing request from client.
if resp.StatusCode == http.StatusTooManyRequests {
return true, nil
}

// Check the response code. We retry on 500-range responses to allow
// the server time to recover, as 500's are typically not permanent
// errors and may relate to outages on the server side. This will catch
Expand Down

0 comments on commit 991b9d0

Please sign in to comment.