From ddefe88ca79b31768e76d497e381f194c45a9ad7 Mon Sep 17 00:00:00 2001 From: Steve Hipwell Date: Fri, 27 Jun 2025 12:00:19 +0100 Subject: [PATCH 1/3] feat: add DisableRateLimitCheck option to client Signed-off-by: Steve Hipwell --- README.md | 73 +++++++++++++++++++++++++++++------------------- github/github.go | 49 +++++++++++++++++++------------- 2 files changed, 75 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index aa269eb5f20..6a0804570c1 100644 --- a/README.md +++ b/README.md @@ -189,54 +189,71 @@ using the installation ID of the GitHub app and authenticate with the OAuth meth ### Rate Limiting ### -GitHub imposes a rate limit on all API clients. Unauthenticated clients are -limited to 60 requests per hour, while authenticated clients can make up to -5,000 requests per hour. The Search API has a custom rate limit. Unauthenticated -clients are limited to 10 requests per minute, while authenticated clients -can make up to 30 requests per minute. To receive the higher rate limit when -making calls that are not issued on behalf of a user, -use `UnauthenticatedRateLimitedTransport`. - -The returned `Response.Rate` value contains the rate limit information +GitHub imposes rate limits on all API clients. The [primary rate limit](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#about-primary-rate-limits) +is the limit to the number of REST API requests that a client can make within a +specific amount of time. This limit helps prevent abuse and denial-of-service +attacks, and ensures that the API remains available for all users. Some +endpoints, like the search endpoints, have more restrictive limits. +Unauthenticated clients may request public data but have a low rate limit, +while authenticated clients have rate limits based on the client +identity. + +In addition to primary rate limits, GitHub enforces [secondary rate limits](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#about-secondary-rate-limits) +in order to prevent abuse and keep the API available for all users. +Secondary rate limits generally limit the number of concurrent requests that a +client can make. + +The client returned `Response.Rate` value contains the rate limit information from the most recent API call. If a recent enough response isn't -available, you can use `RateLimits` to fetch the most up-to-date rate -limit data for the client. +available, you can use the client `RateLimits` service to fetch 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`: +To detect a primary API rate limit error, you can check if the error is a +`RateLimitError`. ```go repos, _, err := client.Repositories.List(ctx, "", nil) -if _, ok := err.(*github.RateLimitError); ok { - log.Println("hit rate limit") +var rateErr *github.RateLimitError +if errors.As(err, &rateError) { + log.Printf("hit primary rate limit, used %d of %d\n", rateErr.Rate.Used, rateErr.rate.Limit) } ``` -Learn more about GitHub rate limiting in -["REST API endpoints for rate limits"](https://docs.github.com/en/rest/rate-limit). - -In addition to these rate limits, GitHub imposes a secondary rate limit on all API clients. -This rate limit prevents clients from making too many concurrent requests. - -To detect an API secondary rate limit error, you can check if its type is `*github.AbuseRateLimitError`: +To detect an API secondary rate limit error, you can check if the error is a +`AbuseRateLimitError`. ```go repos, _, err := client.Repositories.List(ctx, "", nil) -if _, ok := err.(*github.AbuseRateLimitError); ok { - log.Println("hit secondary rate limit") +var rateErr *github.AbuseRateLimitError +if errors.As(err, &rateError) { + log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter) } ``` -Alternatively, you can block until the rate limit is reset by using the `context.WithValue` method: +If you hit the primary rate limit, you can use the `SleepUntilPrimaryRateLimitReset` +method to block until the rate limit is reset. ```go repos, _, err := client.Repositories.List(context.WithValue(ctx, github.SleepUntilPrimaryRateLimitResetWhenRateLimited, true), "", nil) ``` -You can use [gofri/go-github-ratelimit](https://github.com/gofri/go-github-ratelimit) to handle -secondary rate limit sleep-and-retry for you, as well as primary rate limit abuse-prevention and callback triggering. +If you need to make a request even if the rate limit has been hit you can use +the `BypassRateLimitCheck` method to bypass the rate limit check and make the +request anyway. + +```go +repos, _, err := client.Repositories.List(context.WithValue(ctx, github.BypassRateLimitCheck, true), "", nil) +``` + +For more advanced use cases, you can use [gofri/go-github-ratelimit](https://github.com/gofri/go-github-ratelimit) +which provides a middleware (`http.RoundTripper`) that handles both the primary +rate limit and secondary rate limit for the GitHub API. In this case you can +set the client `DisableRateLimitCheck` to `true` so the client doesn't track the rate limit usage. -Learn more about GitHub secondary rate limiting in -["About secondary rate limits"](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#about-secondary-rate-limits). +If the client is an [OAuth app](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#primary-rate-limit-for-oauth-apps) +you can use the apps higher rate limit to request public data by using the +`UnauthenticatedRateLimitedTransport` to make calls as the app instead of as +the user. ### Accepted Status ### diff --git a/github/github.go b/github/github.go index 0e2a9950c29..b57affd604b 100644 --- a/github/github.go +++ b/github/github.go @@ -171,6 +171,11 @@ type Client struct { // User agent used when communicating with the GitHub API. UserAgent string + // DisableRateLimitCheck stops the client checking for rate limits or tracking + // them. This is different to setting BypassRateLimitCheck in the context, + // as that still tracks the rate limits. + DisableRateLimitCheck bool + rateMu sync.Mutex rateLimits [Categories]Rate // Rate limits for the client as determined by the most recent API calls. secondaryRateLimitReset time.Time // Secondary rate limit reset for the client as determined by the most recent API calls. @@ -850,21 +855,26 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ req = withContext(ctx, req) - rateLimitCategory := GetRateLimitCategory(req.Method, req.URL.Path) + rateLimitCategory := CoreCategory - if bypass := ctx.Value(BypassRateLimitCheck); bypass == nil { - // If we've hit rate limit, don't make further requests before Reset time. - if err := c.checkRateLimitBeforeDo(req, rateLimitCategory); err != nil { - return &Response{ - Response: err.Response, - Rate: err.Rate, - }, err - } - // If we've hit a secondary rate limit, don't make further requests before Retry After. - if err := c.checkSecondaryRateLimitBeforeDo(req); err != nil { - return &Response{ - Response: err.Response, - }, err + if !c.DisableRateLimitCheck { + rateLimitCategory = GetRateLimitCategory(req.Method, req.URL.Path) + + if bypass := ctx.Value(BypassRateLimitCheck); bypass == nil { + // If we've hit rate limit, don't make further requests before Reset time. + if err := c.checkRateLimitBeforeDo(req, rateLimitCategory); err != nil { + return &Response{ + Response: err.Response, + Rate: err.Rate, + }, err + } + + // If we've hit a secondary rate limit, don't make further requests before Retry After. + if err := c.checkSecondaryRateLimitBeforeDo(req); err != nil { + return &Response{ + Response: err.Response, + }, err + } } } @@ -894,9 +904,10 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ return response, err } - // Don't update the rate limits if this was a cached response. - // X-From-Cache is set by https://github.com/bartventer/httpcache - if response.Header.Get("X-From-Cache") == "" { + // Don't update the rate limits if the client has rate limits disabled or if + // this was a cached response. The X-From-Cache is set by + // https://github.com/bartventer/httpcache if it's enabled. + if !c.DisableRateLimitCheck && response.Header.Get("X-From-Cache") == "" { c.rateMu.Lock() c.rateLimits[rateLimitCategory] = response.Rate c.rateMu.Unlock() @@ -1586,8 +1597,8 @@ that need to use a higher rate limit associated with your OAuth application. This will add the client id and secret as a base64-encoded string in the format ClientID:ClientSecret and apply it as an "Authorization": "Basic" header. -See https://docs.github.com/rest/#unauthenticated-rate-limited-requests for -more information. +See http://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#primary-rate-limit-for-oauth-apps +for more information. */ type UnauthenticatedRateLimitedTransport struct { // ClientID is the GitHub OAuth client ID of the current application, which From 4bc5ba2ef477bde4d0414cb2900e9e68e45c8319 Mon Sep 17 00:00:00 2001 From: Steve Hipwell Date: Fri, 27 Jun 2025 12:56:56 +0100 Subject: [PATCH 2/3] fixup! feat: add DisableRateLimitCheck option to client --- README.md | 2 +- github/github.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6a0804570c1..794563710b8 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ if errors.As(err, &rateError) { } ``` -To detect an API secondary rate limit error, you can check if the error is a +To detect an API secondary rate limit error, you can check if the error is an `AbuseRateLimitError`. ```go diff --git a/github/github.go b/github/github.go index b57affd604b..481752c5cb9 100644 --- a/github/github.go +++ b/github/github.go @@ -1597,7 +1597,7 @@ that need to use a higher rate limit associated with your OAuth application. This will add the client id and secret as a base64-encoded string in the format ClientID:ClientSecret and apply it as an "Authorization": "Basic" header. -See http://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#primary-rate-limit-for-oauth-apps +See https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#primary-rate-limit-for-oauth-apps for more information. */ type UnauthenticatedRateLimitedTransport struct { From ad32dc452eab4a67c156d706a30f22d49bd0649c Mon Sep 17 00:00:00 2001 From: Steve Hipwell Date: Mon, 30 Jun 2025 13:13:22 +0100 Subject: [PATCH 3/3] fixup! feat: add DisableRateLimitCheck option to client --- README.md | 8 ++--- github/github_test.go | 76 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 794563710b8..f45d3730399 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ To detect a primary API rate limit error, you can check if the error is a repos, _, err := client.Repositories.List(ctx, "", nil) var rateErr *github.RateLimitError if errors.As(err, &rateError) { - log.Printf("hit primary rate limit, used %d of %d\n", rateErr.Rate.Used, rateErr.rate.Limit) + log.Printf("hit primary rate limit, used %d of %d\n", rateErr.Rate.Used, rateErr.rate.Limit) } ``` @@ -225,12 +225,12 @@ To detect an API secondary rate limit error, you can check if the error is an ```go repos, _, err := client.Repositories.List(ctx, "", nil) var rateErr *github.AbuseRateLimitError -if errors.As(err, &rateError) { - log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter) +if errors.As(err, &rateErr) { + log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter) } ``` -If you hit the primary rate limit, you can use the `SleepUntilPrimaryRateLimitReset` +If you hit the primary rate limit, you can use the `SleepUntilPrimaryRateLimitResetWhenRateLimited` method to block until the rate limit is reset. ```go diff --git a/github/github_test.go b/github/github_test.go index 95c7fb2a3c4..412bd26cdae 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -251,6 +251,9 @@ func testNewRequestAndDoFailureCategory(t *testing.T, methodName string, client client.BaseURL.Path = "/api-v3/" client.rateLimits[category].Reset.Time = time.Now().Add(10 * time.Minute) resp, err = f() + if client.DisableRateLimitCheck { + return + } if bypass := resp.Request.Context().Value(BypassRateLimitCheck); bypass != nil { return } @@ -1912,6 +1915,79 @@ func TestDo_rateLimit_abuseRateLimitError_maxDuration(t *testing.T) { } } +// Make network call if client has disabled the rate limit check. +func TestDo_rateLimit_disableRateLimitCheck(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + client.DisableRateLimitCheck = true + + reset := time.Now().UTC().Add(60 * time.Second) + client.rateLimits[CoreCategory] = Rate{Limit: 5000, Remaining: 0, Reset: Timestamp{reset}} + requestCount := 0 + mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) { + requestCount++ + w.Header().Set(headerRateLimit, "5000") + w.Header().Set(headerRateRemaining, "5000") + w.Header().Set(headerRateUsed, "0") + w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) + w.Header().Set(headerRateResource, "core") + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{}`) + }) + req, _ := client.NewRequest("GET", ".", nil) + ctx := context.Background() + resp, err := client.Do(ctx, req, nil) + if err != nil { + t.Errorf("Do returned unexpected error: %v", err) + } + if got, want := resp.StatusCode, http.StatusOK; got != want { + t.Errorf("Response status code = %v, want %v", got, want) + } + if got, want := requestCount, 1; got != want { + t.Errorf("Expected 1 request, got %d", got) + } + if got, want := client.rateLimits[CoreCategory].Remaining, 0; got != want { + t.Errorf("Expected 0 requests remaining, got %d", got) + } +} + +// Make network call if client has bypassed the rate limit check. +func TestDo_rateLimit_bypassRateLimitCheck(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + reset := time.Now().UTC().Add(60 * time.Second) + client.rateLimits[CoreCategory] = Rate{Limit: 5000, Remaining: 0, Reset: Timestamp{reset}} + requestCount := 0 + mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) { + requestCount++ + w.Header().Set(headerRateLimit, "5000") + w.Header().Set(headerRateRemaining, "5000") + w.Header().Set(headerRateUsed, "0") + w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) + w.Header().Set(headerRateResource, "core") + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{}`) + }) + req, _ := client.NewRequest("GET", ".", nil) + ctx := context.Background() + resp, err := client.Do(context.WithValue(ctx, BypassRateLimitCheck, true), req, nil) + if err != nil { + t.Errorf("Do returned unexpected error: %v", err) + } + if got, want := resp.StatusCode, http.StatusOK; got != want { + t.Errorf("Response status code = %v, want %v", got, want) + } + if got, want := requestCount, 1; got != want { + t.Errorf("Expected 1 request, got %d", got) + } + if got, want := client.rateLimits[CoreCategory].Remaining, 5000; got != want { + t.Errorf("Expected 5000 requests remaining, got %d", got) + } +} + func TestDo_noContent(t *testing.T) { t.Parallel() client, mux, _ := setup(t)