-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add DisableRateLimitCheck option to client #3607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -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 an | ||||||||
`AbuseRateLimitError`. | ||||||||
Comment on lines
+222
to
+223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||||||||
|
||||||||
```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, &rateErr) { | ||||||||
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 `SleepUntilPrimaryRateLimitResetWhenRateLimited` | ||||||||
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 | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree. |
||||||||
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 | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
the user. | ||||||||
|
||||||||
### Accepted Status ### | ||||||||
|
||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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, | ||||||||||
Comment on lines
+174
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
// 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 { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add a unit test for this new field? |
||||||||||
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 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 { | ||||||||||
// ClientID is the GitHub OAuth client ID of the current application, which | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? The text in this block appears to be formatted at 80 characters so that's what I've stuck to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need to strictly stick to 80 characters. Many lines in the README already exceed that limit - for example:
go-github/README.md
Line 109 in 0c6bd91
Also, we currently don’t have any linter or formatting rule that enforces a line length limit in the README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block was limited to 80 characters and I've kept it this way other than where there is a link (inspired by the Google Go formatting doc). Given that there is no linting or formatting rules and the README isn't consistent I don't think there is any value in being pedantic here (personally I prefer unbounded line lengths).