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!

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

-	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

+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

+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

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

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

-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

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

+	// 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

- 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 clientJun 27, 2025

gmlewis requested changes Jun 27, 2025
View reviewed changes

Comment threadREADME.md Outdated
Comment threadgithub/github.go Outdated

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 ad32dc4Compare June 30, 2025 12:23

alexandear approved these changes Jun 30, 2025
View reviewed changes

gmlewis merged commit d96da2e into google:masterJun 30, 2025

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

jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
feat: Add DisableRateLimitCheck option to client (google#3607)
19a28a0

	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,

Uh oh!

Conversation

stevehipwell commented Jun 27, 2025

Uh oh!

codecovBot 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecovBot commented Jun 27, 2025•
edited
Loading