feat: Add DisableRateLimitCheck option to client by stevehipwell · Pull Request #3607 · google/go-github

stevehipwell · 2025-06-27T11:11:15Z

This PR adds the DisableRateLimitCheck option to the client to disable all client level rate limit logic. It also updates the documentation on the rate limit logic.

Fixes #3585

@gmlewis apologies if there are quality issues with this PR, my VS Code is broken for this codebase so I put off the work as long as I could.

codecov · 2025-06-27T11:34:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.33%. Comparing base (efe572e) to head (ad32dc4).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3607   +/-   ##
=======================================
  Coverage   91.33%   91.33%           
=======================================
  Files         184      184           
  Lines       16166    16169    +3     
=======================================
+ Hits        14765    14768    +3     
  Misses       1227     1227           
  Partials      174      174

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gmlewis

Thank you, @stevehipwell!
A couple minor tweaks, please, then we should be ready for merging after getting a second LGTM+Approval from any other contributor to this repo.

@alexandear - might you have time for a code review? Thank you!

README.md

github/github.go

stevehipwell · 2025-06-27T11:58:05Z

@gmlewis I've fixed the issues you found.

gmlewis

Thank you, @stevehipwell!
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo.

alexandear · 2025-06-27T14:23:47Z

README.md

-	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)


Suggested change

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)

alexandear · 2025-06-27T14:24:14Z

README.md

+To detect a primary API rate limit error, you can check if the error is a
+`RateLimitError`.


Suggested change

To detect a primary API rate limit error, you can check if the error is a

`RateLimitError`.

To detect a primary API rate limit error, you can check if the error is a `github.RateLimitError`.

Really? The text in this block appears to be formatted at 80 characters so that's what I've stuck to.

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

GitHub Apps authentication can be provided by different pkgs like [bradleyfalzon/ghinstallation](https://github.com/bradleyfalzon/ghinstallation)

Also, we currently don’t have any linter or formatting rule that enforces a line length limit in the README.

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).

alexandear · 2025-06-27T14:29:07Z

README.md

+To detect an API secondary rate limit error, you can check if the error is an
+`AbuseRateLimitError`.


Suggested change

To detect an API secondary rate limit error, you can check if the error is an

`AbuseRateLimitError`.

To detect an API secondary rate limit error, you can check if the error is an `github.AbuseRateLimitError`.

Same as above.

alexandear · 2025-06-27T14:29:21Z

README.md

-	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)


Suggested change

log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter)

log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter)

README.md

alexandear · 2025-06-27T14:32:18Z

README.md

-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


Let's add github. for consistency:

Suggested change

the `BypassRateLimitCheck` method to bypass the rate limit check and make the

the `github.BypassRateLimitCheck` method to bypass the rate limit check and make the

I disagree.

alexandear · 2025-06-27T14:32:45Z

README.md

-["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


Suggested change

`UnauthenticatedRateLimitedTransport` to make calls as the app instead of as

`github.UnauthenticatedRateLimitedTransport` to make calls as the app instead of as

alexandear · 2025-06-27T14:33:18Z

github/github.go

+	// DisableRateLimitCheck stops the client checking for rate limits or tracking
+	// them. This is different to setting BypassRateLimitCheck in the context,


Suggested change

// DisableRateLimitCheck stops the client checking for rate limits or tracking

// them. This is different to setting BypassRateLimitCheck in the context,

// DisableRateLimitCheck stops the client checking for rate limits or tracking them.

// This is different to setting BypassRateLimitCheck in the context,

alexandear · 2025-06-27T14:33:52Z

github/github.go

-			return &Response{
-				Response: err.Response,
-			}, err
+	if !c.DisableRateLimitCheck {


Could we add a unit test for this new field?

alexandear · 2025-06-28T16:26:09Z

Please resolve the conflicts when you have a chance.

Also, feel free to ignore all of my comments except the one about adding a unit test - that one is a must-have.

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

stevehipwell · 2025-06-30T12:28:18Z

@alexandear I've added a unit test for the new disable feature and one for the existing bypass feature that didn't have one.

@gmlewis I rebased to fix the merge conflict, I hope that's OK (I've left the commit structure as it was).

gmlewis · 2025-06-30T15:57:08Z

Thank you, @stevehipwell and @alexandear!
Merging.

gmlewis changed the title ~~feat: add DisableRateLimitCheck option to client~~ feat: Add DisableRateLimitCheck option to client Jun 27, 2025

gmlewis requested changes Jun 27, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

github/github.go Outdated Show resolved Hide resolved

gmlewis added the NeedsReview PR is awaiting a review before merging. label Jun 27, 2025

gmlewis approved these changes Jun 27, 2025

View reviewed changes

alexandear reviewed Jun 27, 2025

View reviewed changes

gmlewis added waiting for reply and removed NeedsReview PR is awaiting a review before merging. labels Jun 30, 2025

stevehipwell added 3 commits June 30, 2025 13:23

feat: add DisableRateLimitCheck option to client

ddefe88

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

fixup! feat: add DisableRateLimitCheck option to client

4bc5ba2

fixup! feat: add DisableRateLimitCheck option to client

ad32dc4

stevehipwell force-pushed the client-rate-limit-bypass branch from 4928013 to ad32dc4 Compare June 30, 2025 12:23

alexandear approved these changes Jun 30, 2025

View reviewed changes

gmlewis merged commit d96da2e into google:master Jun 30, 2025
7 checks passed

stevehipwell deleted the client-rate-limit-bypass branch June 30, 2025 20:21

gmlewis mentioned this pull request Jul 23, 2025

Bump version of go-github to v74.0.0 #3638

Merged

gmlewis removed the waiting for reply label Oct 6, 2025

	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)

		To detect a primary API rate limit error, you can check if the error is a
		`RateLimitError`.

		To detect an API secondary rate limit error, you can check if the error is an
		`AbuseRateLimitError`.

	log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter)
	log.Printf("hit secondary rate limit, retry after %v\n", rateErr.RetryAfter)

	the `BypassRateLimitCheck` method to bypass the rate limit check and make the
	the `github.BypassRateLimitCheck` method to bypass the rate limit check and make the

	`UnauthenticatedRateLimitedTransport` to make calls as the app instead of as
	`github.UnauthenticatedRateLimitedTransport` to make calls as the app instead of as

		// DisableRateLimitCheck stops the client checking for rate limits or tracking
		// them. This is different to setting BypassRateLimitCheck in the context,

Conversation

stevehipwell commented Jun 27, 2025

Uh oh!

codecov bot commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stevehipwell commented Jun 27, 2025

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexandear commented Jun 28, 2025

Uh oh!

stevehipwell commented Jun 30, 2025

Uh oh!

gmlewis commented Jun 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jun 27, 2025 •

edited

Loading