update testing

github_ratelimit/detect.go

@@ -34,7 +34,6 @@ func isRateLimitStatus(statusCode int) bool {
 }
 
 // isSecondaryRateLimit checks whether the response is a legitimate secondary rate limit.
-// it is used to avoid handling primary rate limits and authentic HTTP Forbidden (403) responses.
 func isSecondaryRateLimit(resp *http.Response) bool {
 	if !isRateLimitStatus(resp.StatusCode) {
 		return false
@@ -49,7 +48,7 @@ func isSecondaryRateLimit(resp *http.Response) bool {
 		return false
 	}
 
-	// an authentic HTTP Forbidden (403) response
+	// an authentic HTTP response (not a primary rate limit)
 	defer resp.Body.Close()
 	rawBody, err := io.ReadAll(resp.Body)
 	if err != nil {

github_ratelimit/github_ratelimit_test/ratelimit_injecter.go

@@ -27,13 +27,19 @@ var SecondaryRateLimitDocumentationURLs = []string{
 	`https://docs.github.com/en/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits`,
 }
 
+var SecondaryRateLimitStatusCodes = []int{
+	http.StatusForbidden,
+	http.StatusTooManyRequests,
+}
+
 type SecondaryRateLimitInjecterOptions struct {
 	Every               time.Duration
 	Sleep               time.Duration
 	InvalidBody         bool
 	UseXRateLimit       bool
 	UsePrimaryRateLimit bool
 	DocumentationURL    string
+	HttpStatusCode      int
 }
 
 func NewRateLimitInjecter(base http.RoundTripper, options *SecondaryRateLimitInjecterOptions) (http.RoundTripper, error) {
@@ -130,6 +136,13 @@ func getSecondaryRateLimitBody(documentationURL string) (io.ReadCloser, error) {
 	return io.NopCloser(bytes.NewReader(bodyBytes)), nil
 }
 
+func getHttpStatusCode(statusCode int) int {
+	if statusCode == 0 {
+		return SecondaryRateLimitStatusCodes[0]
+	}
+	return statusCode
+}
+
 func (t *SecondaryRateLimitInjecter) inject(resp *http.Response) (*http.Response, error) {
 	if t.options.UsePrimaryRateLimit {
 		return t.toPrimaryRateLimitResponse(resp), nil
@@ -142,7 +155,7 @@ func (t *SecondaryRateLimitInjecter) inject(resp *http.Response) (*http.Response
 			body = io.NopCloser(bytes.NewReader([]byte(InvalidBodyContent)))
 		}
 
-		resp.StatusCode = http.StatusForbidden
+		resp.StatusCode = getHttpStatusCode(t.options.HttpStatusCode)
 		resp.Body = body
 		if t.options.UseXRateLimit {
 			return t.toXRateLimitResponse(resp), nil

github_ratelimit/github_ratelimit_test/ratelimit_test.go

@@ -103,45 +103,49 @@ func TestSecondaryRateLimit(t *testing.T) {
 	log.Printf("abuse requests: %v/%v (%v%%)\n", asInjecter.AbuseAttempts, requests, abusePrecent)
 }
 
-func TestSecondaryRateLimitBody(t *testing.T) {
+func TestSecondaryRateLimitCombinations(t *testing.T) {
 	t.Parallel()
 	const every = 1 * time.Second
 	const sleep = 1 * time.Second
 
 	for i, docURL := range SecondaryRateLimitDocumentationURLs {
 		docURL := docURL
-		t.Run(fmt.Sprintf("docURL_%d", i), func(t *testing.T) {
-			t.Parallel()
-
-			slept := false
-			callback := func(*github_ratelimit.CallbackContext) {
-				slept = true
-			}
-
-			// test documentation URL
-			i := setupInjecterWithOptions(t, SecondaryRateLimitInjecterOptions{
-				Every:            every,
-				Sleep:            sleep,
-				DocumentationURL: docURL,
+		for j, statusCode := range SecondaryRateLimitStatusCodes {
+			statusCode := statusCode
+			t.Run(fmt.Sprintf("docURL_%d_%d", i, j), func(t *testing.T) {
+				t.Parallel()
+
+				slept := false
+				callback := func(*github_ratelimit.CallbackContext) {
+					slept = true
+				}
+
+				// test documentation URL
+				i := setupInjecterWithOptions(t, SecondaryRateLimitInjecterOptions{
+					Every:            every,
+					Sleep:            sleep,
+					DocumentationURL: docURL,
+					HttpStatusCode:   statusCode,
+				})
+				c, err := github_ratelimit.NewRateLimitWaiterClient(i, github_ratelimit.WithLimitDetectedCallback(callback))
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				// initialize injecter timing
+				_, _ = c.Get("/")
+				waitForNextSleep(i)
+
+				// attempt during rate limit
+				_, err = c.Get("/")
+				if err != nil {
+					t.Fatal(err)
+				}
+				if !slept {
+					t.Fatal(slept)
+				}
 			})
-			c, err := github_ratelimit.NewRateLimitWaiterClient(i, github_ratelimit.WithLimitDetectedCallback(callback))
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			// initialize injecter timing
-			_, _ = c.Get("/")
-			waitForNextSleep(i)
-
-			// attempt during rate limit
-			_, err = c.Get("/")
-			if err != nil {
-				t.Fatal(err)
-			}
-			if !slept {
-				t.Fatal(slept)
-			}
-		})
+		}
 	}
 }
 

github_ratelimit/ratelimit.go

@@ -172,6 +172,10 @@ func parseSecondaryLimitTime(resp *http.Response) *time.Time {
 		return sleepUntil
 	}
 
+	// XXX: per GitHub API docs, we should default to a 60 second sleep time in case the header is missing,
+	//		with an exponential backoff mechanism.
+	//		we may want to implement this in the future (with configurable limits),
+	//		but let's avoid it while there are no known cases of missing headers.
 	return nil
 }
 
@@ -223,7 +227,7 @@ func smoothSleepTime(sleepTime time.Duration) time.Duration {
 	if sleepTime.Milliseconds() == 0 {
 		return sleepTime
 	} else {
-		seconds := (sleepTime.Seconds()) + 1
+		seconds := sleepTime.Seconds() + 1
 		return time.Duration(seconds) * time.Second
 	}
 }