From 5a4a2300867d89989566231569f3cb70b816720b Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Thu, 11 Feb 2016 00:51:40 -0800 Subject: [PATCH] Add RateLimitError type, detect and return it when appropriate. Fixes #152. Add tests for RateLimitError. Refactor populateRate into a more general parseRate. --- github/doc.go | 7 ++++++ github/github.go | 40 ++++++++++++++++++++++++++------- github/github_test.go | 52 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 89 insertions(+), 10 deletions(-) diff --git a/github/doc.go b/github/doc.go index e2066f1f482..9a5a8dfb75f 100644 --- a/github/doc.go +++ b/github/doc.go @@ -65,6 +65,13 @@ been some time since the last API call and other clients have made subsequent requests since then. You can always call RateLimit() directly to get the most up-to-date rate limit data for the client. +To detect an API rate limit error, you can check if its type is *github.RateLimitError: + + repos, _, err := client.Repositories.List("", nil) + if _, ok := err.(*github.RateLimitError); ok { + log.Println("hit rate limit") + } + Learn more about GitHub rate limiting at http://developer.github.com/v3/#rate-limiting. diff --git a/github/github.go b/github/github.go index 8a64455ea76..c6a33e340cc 100644 --- a/github/github.go +++ b/github/github.go @@ -236,7 +236,7 @@ type Response struct { func newResponse(r *http.Response) *Response { response := &Response{Response: r} response.populatePageValues() - response.populateRate() + response.Rate = parseRate(r) return response } @@ -284,19 +284,21 @@ func (r *Response) populatePageValues() { } } -// populateRate parses the rate related headers and populates the response Rate. -func (r *Response) populateRate() { +// parseRate parses the rate related headers. +func parseRate(r *http.Response) Rate { + var rate Rate if limit := r.Header.Get(headerRateLimit); limit != "" { - r.Rate.Limit, _ = strconv.Atoi(limit) + rate.Limit, _ = strconv.Atoi(limit) } if remaining := r.Header.Get(headerRateRemaining); remaining != "" { - r.Rate.Remaining, _ = strconv.Atoi(remaining) + rate.Remaining, _ = strconv.Atoi(remaining) } if reset := r.Header.Get(headerRateReset); reset != "" { if v, _ := strconv.ParseInt(reset, 10, 64); v != 0 { - r.Rate.Reset = Timestamp{time.Unix(v, 0)} + rate.Reset = Timestamp{time.Unix(v, 0)} } } + return rate } // Rate specifies the current rate limit for the client as determined by the @@ -373,6 +375,20 @@ type TwoFactorAuthError ErrorResponse func (r *TwoFactorAuthError) Error() string { return (*ErrorResponse)(r).Error() } +// RateLimitError occurs when GitHub returns 403 Forbidden response with a rate limit +// remaining value of 0, and error message starts with "API rate limit exceeded for ". +type RateLimitError struct { + Rate Rate // Rate specifies last known rate limit for the client + Response *http.Response // HTTP response that caused this error + Message string `json:"message"` // error message +} + +func (r *RateLimitError) Error() string { + return fmt.Sprintf("%v %v: %d %v; rate reset in %v", + r.Response.Request.Method, sanitizeURL(r.Response.Request.URL), + r.Response.StatusCode, r.Message, r.Rate.Reset.Time.Sub(time.Now())) +} + // 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 { @@ -427,10 +443,18 @@ func CheckResponse(r *http.Response) error { if err == nil && data != nil { json.Unmarshal(data, errorResponse) } - if r.StatusCode == http.StatusUnauthorized && strings.HasPrefix(r.Header.Get(headerOTP), "required") { + 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" && strings.HasPrefix(errorResponse.Message, "API rate limit exceeded for "): + return &RateLimitError{ + Rate: parseRate(r), + Response: errorResponse.Response, + Message: errorResponse.Message, + } + default: + return errorResponse } - return errorResponse } // parseBoolResponse determines the boolean result from a GitHub API response. diff --git a/github/github_test.go b/github/github_test.go index 00fa63edf33..2ffd7d16cc5 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -389,8 +389,11 @@ func TestDo_rateLimit(t *testing.T) { } req, _ := client.NewRequest("GET", "/", nil) - client.Do(req, nil) + _, err := client.Do(req, nil) + if err != nil { + t.Errorf("Do returned unexpected error: %v", err) + } if got, want := client.Rate().Limit, 60; got != want { t.Errorf("Client rate limit = %v, want %v", got, want) } @@ -416,8 +419,14 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { }) req, _ := client.NewRequest("GET", "/", nil) - client.Do(req, nil) + _, err := client.Do(req, nil) + if err == nil { + t.Error("Expected error to be returned.") + } + if _, ok := err.(*RateLimitError); ok { + t.Errorf("Did not expect a *RateLimitError error; got %#v.", err) + } if got, want := client.Rate().Limit, 60; got != want { t.Errorf("Client rate limit = %v, want %v", got, want) } @@ -430,6 +439,45 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { } } +// Ensure *RateLimitError is returned when API rate limit is exceeded. +func TestDo_rateLimit_rateLimitError(t *testing.T) { + setup() + defer teardown() + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.Header().Add(headerRateLimit, "60") + w.Header().Add(headerRateRemaining, "0") + w.Header().Add(headerRateReset, "1372700873") + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(http.StatusForbidden) + fmt.Fprintln(w, `{ + "message": "API rate limit exceeded for xxx.xxx.xxx.xxx. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", + "documentation_url": "https://developer.github.com/v3/#rate-limiting" +}`) + }) + + req, _ := client.NewRequest("GET", "/", nil) + _, err := client.Do(req, nil) + + if err == nil { + t.Error("Expected error to be returned.") + } + rateLimitErr, ok := err.(*RateLimitError) + if !ok { + t.Fatalf("Expected a *RateLimitError error; got %#v.", err) + } + if got, want := rateLimitErr.Rate.Limit, 60; got != want { + t.Errorf("rateLimitErr rate limit = %v, want %v", got, want) + } + if got, want := rateLimitErr.Rate.Remaining, 0; got != want { + t.Errorf("rateLimitErr rate remaining = %v, want %v", got, want) + } + reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC) + if rateLimitErr.Rate.Reset.UTC() != reset { + t.Errorf("rateLimitErr rate reset = %v, want %v", client.Rate().Reset, reset) + } +} + func TestDo_noContent(t *testing.T) { setup() defer teardown()