Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add and detect AbuseRateLimitError. #441

Merged
merged 1 commit into from
Oct 11, 2016
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
32 changes: 32 additions & 0 deletions github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,24 @@ func (r *RateLimitError) Error() string {
r.Response.StatusCode, r.Message, r.Rate.Reset.Time.Sub(time.Now()))
}

// AbuseRateLimitError occurs when GitHub returns 403 Forbidden response with the
// "documentation_url" field value equal to "https://developer.github.com/v3#abuse-rate-limits".
type AbuseRateLimitError struct {
Response *http.Response // HTTP response that caused this error
Message string `json:"message"` // error message

// RetryAfter is provided with some abuse rate limit errors. If present,
// it is the amount of time that the client should wait before retrying.
// Otherwise, the client should try again later (after an unspecified amount of time).
RetryAfter *time.Duration
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially tried to use a value rather than pointer:

RetryAfter time.Duration

But, I changed it to be a pointer to be able to differentiate between a missing Retry-After header (which means "retry later after an unspecified time") vs a present Retry-After header with value of 0 seconds. I have no evidence/proof that 0 is an impossible value, that's why I couldn't use it as a sentinel to mean "unspecified time". If 0 were to come up in reality, it means "retry right now", which is very close to "retry after 1 second", and very different from "retry after unspecified time".

}

func (r *AbuseRateLimitError) Error() string {
return fmt.Sprintf("%v %v: %d %v",
r.Response.Request.Method, sanitizeURL(r.Response.Request.URL),
r.Response.StatusCode, r.Message)
}

// sanitizeURL redacts the client_secret parameter from the URL which may be
// exposed to the user, specifically in the ErrorResponse error message.
func sanitizeURL(uri *url.URL) *url.URL {
Expand Down Expand Up @@ -564,6 +582,20 @@ func CheckResponse(r *http.Response) error {
Response: errorResponse.Response,
Message: errorResponse.Message,
}
case r.StatusCode == http.StatusForbidden && errorResponse.DocumentationURL == "https://developer.github.com/v3#abuse-rate-limits":
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I matched based on the documentation_url value rather than a prefix of the message is because this seems more reliable.

I know there are multiple different types of abuse rate limit errors, and they have different message text. They may have the same prefix, but I don't know for sure what are all the possible messages. If the documentation_url value is "https://developer.github.com/v3#abuse-rate-limits" (note the "abuse" in the URL, it's not https://developer.github.com/v3/#rate-limiting), then it seems quite reasonable to expect this is an abuse rate limit error rather than something else.

If there are improvement suggestions, I'd be glad to hear them.

abuseRateLimitError := &AbuseRateLimitError{
Response: errorResponse.Response,
Message: errorResponse.Message,
}
if v := r.Header["Retry-After"]; len(v) > 0 {
// According to GitHub support, the "Retry-After" header value will be
// an integer which represents the number of seconds that one should
// wait before resuming making requests.
retryAfterSeconds, _ := strconv.ParseInt(v[0], 10, 64) // Error handling is noop.
retryAfter := time.Duration(retryAfterSeconds) * time.Second
abuseRateLimitError.RetryAfter = &retryAfter
}
return abuseRateLimitError
default:
return errorResponse
}
Expand Down
65 changes: 65 additions & 0 deletions github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,71 @@ func TestDo_rateLimit_noNetworkCall(t *testing.T) {
}
}

// Ensure *AbuseRateLimitError is returned when the response indicates that
// the client has triggered an abuse detection mechanism.
func TestDo_rateLimit_abuseRateLimitError(t *testing.T) {
setup()
defer teardown()

mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json; charset=utf-8")
w.WriteHeader(http.StatusForbidden)
// When the abuse rate limit error is of the "temporarily blocked from content creation" type,
// there is no "Retry-After" header.
fmt.Fprintln(w, `{
"message": "You have triggered an abuse detection mechanism and have been temporarily blocked from content creation. Please retry your request again later.",
"documentation_url": "https://developer.github.com/v3#abuse-rate-limits"
}`)
})

req, _ := client.NewRequest("GET", "/", nil)
_, err := client.Do(req, nil)

if err == nil {
t.Error("Expected error to be returned.")
}
abuseRateLimitErr, ok := err.(*AbuseRateLimitError)
if !ok {
t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err)
}
if got, want := abuseRateLimitErr.RetryAfter, (*time.Duration)(nil); got != want {
t.Errorf("abuseRateLimitErr RetryAfter = %v, want %v", got, want)
}
}

// Ensure *AbuseRateLimitError.RetryAfter is parsed correctly.
func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) {
setup()
defer teardown()

mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json; charset=utf-8")
w.Header().Set("Retry-After", "123") // Retry after value of 123 seconds.
w.WriteHeader(http.StatusForbidden)
fmt.Fprintln(w, `{
"message": "You have triggered an abuse detection mechanism ...",
"documentation_url": "https://developer.github.com/v3#abuse-rate-limits"
}`)
})

req, _ := client.NewRequest("GET", "/", nil)
_, err := client.Do(req, nil)

if err == nil {
t.Error("Expected error to be returned.")
}
abuseRateLimitErr, ok := err.(*AbuseRateLimitError)
if !ok {
t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err)
}
if abuseRateLimitErr.RetryAfter == nil {
t.Fatalf("abuseRateLimitErr RetryAfter is nil, expected not-nil")
}
if got, want := *abuseRateLimitErr.RetryAfter, 123*time.Second; got != want {
t.Errorf("abuseRateLimitErr RetryAfter = %v, want %v", got, want)
}
}

func TestDo_noContent(t *testing.T) {
setup()
defer teardown()
Expand Down