Merge pull request #17 from bitso/fix/github-documentation-url

Check documentation url suffix

github_ratelimit/detect.go

@@ -14,12 +14,16 @@ type SecondaryRateLimitBody struct {
 }
 
 const (
-	SecondaryRateLimitMessage          = `You have exceeded a secondary rate limit`
-	SecondaryRateLimitDocumentationURL = `https://docs.github.com/rest/overview/resources-in-the-rest-api#secondary-rate-limits`
+	SecondaryRateLimitMessage           = `You have exceeded a secondary rate limit`
+	SecondaryRateLimitDocumentationPath = `/rest/overview/resources-in-the-rest-api#secondary-rate-limits`
 )
 
+// IsSecondaryRateLimit checks whether the response is a legitimate secondary rate limit.
+// It checks the prefix of the message and the suffix of the documentation URL in the response body in case
+// the message or documentation URL is modified in the future.
+// https://docs.github.com/en/rest/overview/resources-in-the-rest-api#secondary-rate-limits
 func (s SecondaryRateLimitBody) IsSecondaryRateLimit() bool {
-	return strings.HasPrefix(s.Message, SecondaryRateLimitMessage) && s.DocumentURL == SecondaryRateLimitDocumentationURL
+	return strings.HasPrefix(s.Message, SecondaryRateLimitMessage) && strings.HasSuffix(s.DocumentURL, SecondaryRateLimitDocumentationPath)
 }
 
 // isSecondaryRateLimit checks whether the response is a legitimate secondary rate limit.

github_ratelimit/github_ratelimit_test/ratelimit_injecter.go

@@ -17,12 +17,19 @@ const (
 	InvalidBodyContent = `{"message": "not as expected"}`
 )
 
+const (
+	SecondaryRateLimitMessage                   = `You have exceeded a secondary rate limit. Please wait a few minutes before you try again.`
+	SecondaryRateLimitDocumentationURL          = `https://docs.github.com/rest/overview/resources-in-the-rest-api#secondary-rate-limits`
+	SecondaryRateLimitAlternateDocumentationURL = `https://docs.github.com/free-pro-team@latest/rest/overview/resources-in-the-rest-api#secondary-rate-limits`
+)
+
 type SecondaryRateLimitInjecterOptions struct {
-	Every               time.Duration
-	Sleep               time.Duration
-	UseXRateLimit       bool
-	UsePrimaryRateLimit bool
-	InvalidBody         bool
+	Every                        time.Duration
+	Sleep                        time.Duration
+	InvalidBody                  bool
+	UseXRateLimit                bool
+	UsePrimaryRateLimit          bool
+	UseAlternateDocumentationURL bool
 }
 
 func NewRateLimitInjecter(base http.RoundTripper, options *SecondaryRateLimitInjecterOptions) (http.RoundTripper, error) {
@@ -102,10 +109,16 @@ func (r *SecondaryRateLimitInjecter) NextSleepStart() time.Time {
 	return r.blockUntil.Add(r.options.Every)
 }
 
-func getSecondaryRateLimitBody() (io.ReadCloser, error) {
+func getSecondaryRateLimitBody(useAlternatateDocumentationURL bool) (io.ReadCloser, error) {
+	documentURL := SecondaryRateLimitDocumentationURL
+
+	if useAlternatateDocumentationURL {
+		documentURL = SecondaryRateLimitAlternateDocumentationURL
+	}
+
 	body := github_ratelimit.SecondaryRateLimitBody{
-		Message:     github_ratelimit.SecondaryRateLimitMessage,
-		DocumentURL: github_ratelimit.SecondaryRateLimitDocumentationURL,
+		Message:     SecondaryRateLimitMessage,
+		DocumentURL: documentURL,
 	}
 	bodyBytes, err := json.Marshal(body)
 	if err != nil {
@@ -119,7 +132,7 @@ func (t *SecondaryRateLimitInjecter) inject(resp *http.Response) (*http.Response
 	if t.options.UsePrimaryRateLimit {
 		return t.toPrimaryRateLimitResponse(resp), nil
 	} else {
-		body, err := getSecondaryRateLimitBody()
+		body, err := getSecondaryRateLimitBody(t.options.UseAlternateDocumentationURL)
 		if err != nil {
 			return nil, err
 		}

github_ratelimit/github_ratelimit_test/ratelimit_test.go

@@ -103,6 +103,66 @@ func TestSecondaryRateLimit(t *testing.T) {
 	log.Printf("abuse requests: %v/%v (%v%%)\n", asInjecter.AbuseAttempts, requests, abusePrecent)
 }
 
+func TestSecondaryRateLimitBody(t *testing.T) {
+	t.Parallel()
+	const every = 1 * time.Second
+	const sleep = 1 * time.Second
+
+	slept := false
+	callback := func(*github_ratelimit.CallbackContext) {
+		slept = true
+	}
+
+	// test documentation URL
+	i := setupInjecterWithOptions(t, SecondaryRateLimitInjecterOptions{
+		Every:                        every,
+		Sleep:                        sleep,
+		UseAlternateDocumentationURL: false,
+	})
+	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)
+	}
+
+	// test alternate documentation URL
+	slept = false
+	i = setupInjecterWithOptions(t, SecondaryRateLimitInjecterOptions{
+		Every:                        every,
+		Sleep:                        sleep,
+		UseAlternateDocumentationURL: true,
+	})
+	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)
+	}
+}
+
 func TestSingleSleepLimit(t *testing.T) {
 	t.Parallel()
 	const every = 1 * time.Second
@@ -335,7 +395,7 @@ func TestHTTPForbiddenIgnored(t *testing.T) {
 	_, _ = c.Get("/")
 	waitForNextSleep(i)
 
-	// attempt during rate limit (using invalid body, so the injction is of HTTP Foribdden)
+	// attempt during rate limit (using invalid body, so the injection is of HTTP Forbidden)
 	resp, err := c.Get("/")
 	if err != nil {
 		t.Fatal(err)
@@ -344,9 +404,9 @@ func TestHTTPForbiddenIgnored(t *testing.T) {
 		t.Fatal(slept)
 	}
 
-	if invaidBody, err := IsInvalidBody(resp); err != nil {
+	if invalidBody, err := IsInvalidBody(resp); err != nil {
 		t.Fatal(err)
-	} else if !invaidBody {
+	} else if !invalidBody {
 		t.Fatalf("expected invalid body")
 	}
 }