Skip to content

Fixed a bug when Retry-After is 0, it should use min wait time. #170

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

Open
wants to merge 1 commit into
base: main
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
6 changes: 5 additions & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,11 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response)
if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode == http.StatusServiceUnavailable {
if s, ok := resp.Header["Retry-After"]; ok {
if sleep, err := strconv.ParseInt(s[0], 10, 64); err == nil {
return time.Second * time.Duration(sleep)
if sleep > 0 {
return time.Second * time.Duration(sleep)
} else {
return min
}
}
}
}
Expand Down
64 changes: 36 additions & 28 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,37 +640,45 @@ func TestClient_CheckRetry(t *testing.T) {

func TestClient_DefaultBackoff(t *testing.T) {
for _, code := range []int{http.StatusTooManyRequests, http.StatusServiceUnavailable} {
t.Run(fmt.Sprintf("http_%d", code), func(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Retry-After", "2")
http.Error(w, fmt.Sprintf("test_%d_body", code), code)
}))
defer ts.Close()

client := NewClient()

var retryAfter time.Duration
retryable := false

client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) {
retryable, _ = DefaultRetryPolicy(context.Background(), resp, err)
retryAfter = DefaultBackoff(client.RetryWaitMin, client.RetryWaitMax, 1, resp)
return false, nil
}
for _, responseRetryAfter := range []string{"0", "2"} {
t.Run(fmt.Sprintf("http_%d", code), func(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Retry-After", responseRetryAfter)
http.Error(w, fmt.Sprintf("test_%d_body", code), code)
}))
defer ts.Close()

client := NewClient()

var retryAfter time.Duration
retryable := false

client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) {
retryable, _ = DefaultRetryPolicy(context.Background(), resp, err)
retryAfter = DefaultBackoff(client.RetryWaitMin, client.RetryWaitMax, 1, resp)
return false, nil
}

_, err := client.Get(ts.URL)
if err != nil {
t.Fatalf("expected no errors since retryable")
}
_, err := client.Get(ts.URL)
if err != nil {
t.Fatalf("expected no errors since retryable")
}

if !retryable {
t.Fatal("Since the error is recoverable, the default policy shall return true")
}
if !retryable {
t.Fatal("Since the error is recoverable, the default policy shall return true")
}

if retryAfter != 2*time.Second {
t.Fatalf("The header Retry-After specified 2 seconds, and shall not be %d seconds", retryAfter/time.Second)
}
})
if responseRetryAfter == "0" {
if retryAfter != client.RetryWaitMin {
t.Fatalf("The header Retry-After specified %d seconds, and shall not be %d seconds", client.RetryWaitMin/time.Second, retryAfter/time.Second)
}
} else {
if retryAfter != 2*time.Second {
t.Fatalf("The header Retry-After specified 2 seconds, and shall not be %d seconds", retryAfter/time.Second)
}
}
})
}
}
}

Expand Down