Inifinite retries on 429 is no longer the default

CHANGELOG.md

@@ -1,5 +1,15 @@
 # CHANGELOG
 
+## v14.0.0
+
+## Breaking Changes
+
+- By default, the `DoRetryForStatusCodes` functions will no longer infinitely retry a request when the response returns an HTTP status code of 429 (StatusTooManyRequests).  To opt in to the old behavior set `autorest.Count429AsRetry` to `false`.
+
+## New Features
+
+- Variable `autorest.Max429Delay` can be used to control the maximum delay between retries when a 429 is received with no `Retry-After` header.  The default is zero which means there is no cap.
+
 ## v13.4.0
 
 ## New Features

autorest/sender.go

@@ -257,14 +257,20 @@ func DoRetryForAttempts(attempts int, backoff time.Duration) SendDecorator {
 	}
 }
 
+// Count429AsRetry indicates that a 429 response should be included as a retry attempt.
+var Count429AsRetry = true
+
+// Max429Delay is the maximum duration to wait between retries on a 429 if no Retry-After header was received.
+var Max429Delay time.Duration
+
 // DoRetryForStatusCodes returns a SendDecorator that retries for specified statusCodes for up to the specified
 // number of attempts, exponentially backing off between requests using the supplied backoff
 // time.Duration (which may be zero). Retrying may be canceled by cancelling the context on the http.Request.
 // NOTE: Code http.StatusTooManyRequests (429) will *not* be counted against the number of attempts.
 func DoRetryForStatusCodes(attempts int, backoff time.Duration, codes ...int) SendDecorator {
 	return func(s Sender) Sender {
 		return SenderFunc(func(r *http.Request) (*http.Response, error) {
-			return doRetryForStatusCodesImpl(s, r, false, attempts, backoff, 0, codes...)
+			return doRetryForStatusCodesImpl(s, r, Count429AsRetry, attempts, backoff, 0, codes...)
 		})
 	}
 }
@@ -276,7 +282,7 @@ func DoRetryForStatusCodes(attempts int, backoff time.Duration, codes ...int) Se
 func DoRetryForStatusCodesWithCap(attempts int, backoff, cap time.Duration, codes ...int) SendDecorator {
 	return func(s Sender) Sender {
 		return SenderFunc(func(r *http.Request) (*http.Response, error) {
-			return doRetryForStatusCodesImpl(s, r, true, attempts, backoff, cap, codes...)
+			return doRetryForStatusCodesImpl(s, r, Count429AsRetry, attempts, backoff, cap, codes...)
 		})
 	}
 }
@@ -297,11 +303,10 @@ func doRetryForStatusCodesImpl(s Sender, r *http.Request, count429 bool, attempt
 			return resp, err
 		}
 		delayed := DelayWithRetryAfter(resp, r.Context().Done())
-		// enforce a 2 minute cap between requests when 429 status codes are
-		// not going to be counted as an attempt and when the cap is 0.
-		// this should only happen in the absence of a retry-after header.
-		if !count429 && cap == 0 {
-			cap = 2 * time.Minute
+		// if this was a 429 set the delay cap as specified.
+		// applicable only in the absence of a retry-after header.
+		if resp != nil && resp.StatusCode == http.StatusTooManyRequests {
+			cap = Max429Delay
 		}
 		if !delayed && !DelayForBackoffWithCap(backoff, cap, delayCount, r.Context().Done()) {
 			return resp, r.Context().Err()

autorest/sender_test.go

@@ -798,6 +798,8 @@ func newAcceptedResponse() *http.Response {
 }
 
 func TestDelayWithRetryAfterWithSuccess(t *testing.T) {
+	Count429AsRetry = false
+	defer func() { Count429AsRetry = true }()
 	after, retries := 2, 2
 	totalSecs := after * retries
 
@@ -821,8 +823,44 @@ func TestDelayWithRetryAfterWithSuccess(t *testing.T) {
 		ByDiscardingBody(),
 		ByClosing())
 
+	if r.StatusCode != http.StatusOK {
+		t.Fatalf("autorest: Sender#DelayWithRetryAfterWithSuccess -- got status code %d, wanted 200", r.StatusCode)
+	}
 	if client.Attempts() != 3 {
-		t.Fatalf("autorest: Sender#DelayWithRetryAfter -- Got: StatusCode %v in %v attempts; Want: StatusCode 200 OK in 2 attempts -- ",
+		t.Fatalf("autorest: Sender#DelayWithRetryAfterWithSuccess -- Got: StatusCode %v in %v attempts; Want: StatusCode 200 OK in 2 attempts -- ",
+			r.Status, client.Attempts()-1)
+	}
+}
+
+func TestDelayWithRetryAfterWithFail(t *testing.T) {
+	after, retries := 2, 2
+	totalSecs := after * retries
+
+	client := mocks.NewSender()
+	resp := mocks.NewResponseWithStatus("429 Too many requests", http.StatusTooManyRequests)
+	mocks.SetResponseHeader(resp, "Retry-After", fmt.Sprintf("%v", after))
+	client.AppendAndRepeatResponse(resp, retries)
+	client.AppendResponse(mocks.NewResponseWithStatus("200 OK", http.StatusOK))
+
+	d := time.Second * time.Duration(totalSecs)
+	start := time.Now()
+	r, _ := SendWithSender(client, mocks.NewRequest(),
+		DoRetryForStatusCodes(1, time.Duration(time.Second), http.StatusTooManyRequests),
+	)
+
+	if time.Since(start) < d {
+		t.Fatal("autorest: DelayWithRetryAfter failed stopped too soon")
+	}
+
+	Respond(r,
+		ByDiscardingBody(),
+		ByClosing())
+
+	if r.StatusCode != http.StatusTooManyRequests {
+		t.Fatalf("autorest: Sender#DelayWithRetryAfterWithFail -- got status code %d, wanted 429", r.StatusCode)
+	}
+	if client.Attempts() != 2 {
+		t.Fatalf("autorest: Sender#DelayWithRetryAfterWithFail -- Got: StatusCode %v in %v attempts; Want: StatusCode 429 OK in 1 attempt -- ",
 			r.Status, client.Attempts()-1)
 	}
 }
@@ -942,6 +980,8 @@ func TestDoRetryForStatusCodes_NilResponseFatalError(t *testing.T) {
 }
 
 func TestDoRetryForStatusCodes_Cancel429(t *testing.T) {
+	Count429AsRetry = false
+	defer func() { Count429AsRetry = true }()
 	retries := 6
 	client := mocks.NewSender()
 	resp := mocks.NewResponseWithStatus("429 Too many requests", http.StatusTooManyRequests)

autorest/version.go

@@ -19,7 +19,7 @@ import (
 	"runtime"
 )
 
-const number = "v13.4.0"
+const number = "v14.0.0"
 
 var (
 	userAgent = fmt.Sprintf("Go/%s (%s-%s) go-autorest/%s",