Skip to content
Merged
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
9 changes: 7 additions & 2 deletions github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -1451,13 +1451,18 @@ func CheckResponse(r *http.Response) error {
switch {
case r.StatusCode == http.StatusUnauthorized && strings.HasPrefix(r.Header.Get(headerOTP), "required"):
return (*TwoFactorAuthError)(errorResponse)
case r.StatusCode == http.StatusForbidden && r.Header.Get(headerRateRemaining) == "0":
// Primary rate limit exceeded: GitHub returns 403 or 429 with X-RateLimit-Remaining: 0
// See: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
case (r.StatusCode == http.StatusForbidden || r.StatusCode == http.StatusTooManyRequests) &&
r.Header.Get(headerRateRemaining) == "0":
return &RateLimitError{
Rate: parseRate(r),
Response: errorResponse.Response,
Message: errorResponse.Message,
}
case r.StatusCode == http.StatusForbidden &&
// Secondary rate limit exceeded: GitHub returns 403 or 429 with specific documentation_url
// See: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#about-secondary-rate-limits
case (r.StatusCode == http.StatusForbidden || r.StatusCode == http.StatusTooManyRequests) &&
(strings.HasSuffix(errorResponse.DocumentationURL, "#abuse-rate-limits") ||
strings.HasSuffix(errorResponse.DocumentationURL, "secondary-rate-limits")):
abuseRateLimitError := &AbuseRateLimitError{
Expand Down
71 changes: 71 additions & 0 deletions github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2182,6 +2182,77 @@ func TestCheckResponse_AbuseRateLimit(t *testing.T) {
}
}

// TestCheckResponse_RateLimit_TooManyRequests tests that HTTP 429 with
// X-RateLimit-Remaining: 0 is correctly detected as RateLimitError.
// GitHub API can return either 403 or 429 for rate limiting.
// See: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
func TestCheckResponse_RateLimit_TooManyRequests(t *testing.T) {
t.Parallel()
res := &http.Response{
Request: &http.Request{},
StatusCode: http.StatusTooManyRequests,
Header: http.Header{},
Body: io.NopCloser(strings.NewReader(`{"message":"m",
"documentation_url": "url"}`)),
}
res.Header.Set(headerRateLimit, "60")
res.Header.Set(headerRateRemaining, "0")
res.Header.Set(headerRateUsed, "60")
res.Header.Set(headerRateReset, "243424")
res.Header.Set(headerRateResource, "core")

var err *RateLimitError
errors.As(CheckResponse(res), &err)

if err == nil {
t.Error("Expected error response.")
}

want := &RateLimitError{
Rate: parseRate(res),
Response: res,
Message: "m",
}
if !errors.Is(err, want) {
t.Errorf("Error = %#v, want %#v", err, want)
}
}

// TestCheckResponse_AbuseRateLimit_TooManyRequests tests that HTTP 429 with
// secondary rate limit documentation_url is correctly detected as AbuseRateLimitError.
// GitHub API can return either 403 or 429 for secondary rate limits.
// See: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#about-secondary-rate-limits
func TestCheckResponse_AbuseRateLimit_TooManyRequests(t *testing.T) {
t.Parallel()
res := &http.Response{
Request: &http.Request{},
StatusCode: http.StatusTooManyRequests,
Header: http.Header{},
Body: io.NopCloser(strings.NewReader(`{"message":"m",
"documentation_url": "https://docs.github.com/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits"}`)),
}
res.Header.Set(headerRetryAfter, "60")

var err *AbuseRateLimitError
errors.As(CheckResponse(res), &err)

if err == nil {
t.Fatal("Expected error response.")
}

if err.Response != res {
t.Errorf("Response = %v, want %v", err.Response, res)
}
if err.Message != "m" {
t.Errorf("Message = %q, want %q", err.Message, "m")
}
if err.RetryAfter == nil {
t.Error("Expected RetryAfter to be set")
} else if *err.RetryAfter != 60*time.Second {
t.Errorf("RetryAfter = %v, want %v", *err.RetryAfter, 60*time.Second)
}
}

func TestCheckResponse_RedirectionError(t *testing.T) {
t.Parallel()
urlStr := "/foo/bar"
Expand Down
Loading