Merge pull request #1993 from RicePatrick/add-retry-exponential-backoff

Add an exponential backoff to the retry function

Author: svanharmelen

gitlab.go

@@ -24,6 +24,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"math/rand"
 	"mime/multipart"
 	"net/http"
@@ -510,7 +511,7 @@ func (c *Client) retryHTTPBackoff(min, max time.Duration, attemptNum int, resp *
 // min and max are mainly used for bounding the jitter that will be added to
 // the reset time retrieved from the headers. But if the final wait time is
 // less then min, min will be used instead.
-func rateLimitBackoff(min, max time.Duration, _ int, resp *http.Response) time.Duration {
+func rateLimitBackoff(min, max time.Duration, attemptNum int, resp *http.Response) time.Duration {
 	// rnd is used to generate pseudo-random numbers.
 	rnd := rand.New(rand.NewSource(time.Now().UnixNano()))
 
@@ -525,6 +526,11 @@ func rateLimitBackoff(min, max time.Duration, _ int, resp *http.Response) time.D
 					min = wait
 				}
 			}
+		} else {
+			// In case the RateLimit-Reset header is not set, back off an additional
+			// 100% exponentially. With the default milliseconds being set to 100 for
+			// `min`, this makes the 5th retry wait 3.2 seconds (3,200 ms) by default.
+			min = time.Duration(float64(min) * math.Pow(2, float64(attemptNum)))
 		}
 	}
 

gitlab_test.go

@@ -418,3 +418,39 @@ func TestPaginationPopulatePageValuesKeyset(t *testing.T) {
 		}
 	}
 }
+
+func TestExponentialBackoffLogic(t *testing.T) {
+	// Can't use the default `setup` because it disabled the backoff
+	mux := http.NewServeMux()
+	server := httptest.NewServer(mux)
+	t.Cleanup(server.Close)
+	client, err := NewClient("",
+		WithBaseURL(server.URL),
+	)
+	if err != nil {
+		t.Fatalf("Failed to create client: %v", err)
+	}
+
+	// Create a method that returns 429
+	mux.HandleFunc("/api/v4/projects/1", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, http.MethodGet)
+		w.WriteHeader(http.StatusTooManyRequests)
+	})
+
+	// Measure the time at the start of the test
+	start := time.Now()
+
+	// Send a request (which will get a bunch of 429s)
+	// None of the responses matter, so ignore them all
+	_, resp, _ := client.Projects.GetProject(1, nil)
+	end := time.Now()
+
+	// The test should run for _at least_ 3,200 milliseconds
+	duration := float64(end.Sub(start))
+	if duration < float64(3200*time.Millisecond) {
+		t.Fatal("Wait was shorter than expected. Expected a minimum of 5 retries taking 3200 milliseconds, got:", duration)
+	}
+	if resp.StatusCode != 429 {
+		t.Fatal("Expected to get a 429 code given the server is hard-coded to return this. Received instead:", resp.StatusCode)
+	}
+}