Handle Callback without waiting/reattempting?

[johnmcollier]

I'll preface this with saying my use case here might be a little niche.

I'm using go-github-ratelimit in a Kubernetes controller where I have a pool of go-github clients allocated. Because of the nature of the controller, it's not feasible to wait/block for significant periods of time. Instead, it's preferable for the controller to requeue and reattempt later.

I've defined a callback function that cordons off the affect client until the wait period is over, and allows the HTTP request to fail/return an error. At which point, the controller will requeue and reattempt with a different client. However, go-github-ratelimit is still waiting outside of the callback function, and reattempting the request.

Basically what I'm wondering is, is it possible to use go-github-ratelimit to catch the Secondary rate limit error, execute some callback function, and then allow the request to fail, without waiting?

My current alternative is to manually check the response from go-github for a secondary rate limit error and do the necessary steps there, but being able to do this in a package would be nice 🙂

[johnmcollier]

Just realized that I can achieve what I need by using github_ratelimit.WithSingleSleepLimit and passing a 0 (or small value) to the sleep limit.

The callback context's sleepUntil value will still be based on what was set in the Retry-After / x-ratelimit-reset header, and I can use that in my callback if needed.

I think this issue can be closed, unless there's any problems with this approach?

[gofri]

Just realized that I can achieve what I need by using github_ratelimit.WithSingleSleepLimit and passing a 0 (or small value) to the sleep limit.

Yep, that's exactly what I was about to suggest. Just note that you need to use the onSingleLimitExceeded callback now (onLimitDetected isn't triggered if the sleep duration is above the limit). edit: just noticed you already use that - perfect.

Also note that rather than getting an error from the ratelimit client, it'll return the original response. Assuming you use go-github as the client, you'll get an AbuseRateLimitError from it. btw, it doesn't support x-ratelimit-reset for secondary rate limit yet, which I know you're facing - so don't rely on its RetryAfter field for the sleep (it'll still return an AbuseRateLimitError though).

I was planning to add an option to get an error rather than sleep-and-retry as part of the breaking changes for V2, but I can implement that part without breaking anything - so let's keep this issue open for that purpose.

---

One last thing - In the original notification I got in the mail for the issue in appstudio, you mentioned: In my tests, I'm finding that time.Until(*cbContext.SleepUntil) is a negative number, and the callback function is already sleeping before I call my time.Sleep This should never happen: the callback should be called before the sleep. I think I'll add some tests for the callback context. Anyway, you edited this comment so I hope that means that it was because something was wrong in the testing, but I want to make sure that there's no real issue lurking here (so please elaborate).

[johnmcollier]

Thanks! That all sounds good to me. And good to know about https://github.com/google/go-github/pull/2775/files, I'll keep an eye on that.

Anyway, you edited this comment so I hope that means that it was because something was wrong in the testing, but I want to make sure that there's no real issue lurking here (so please elaborate).

Yup, it wasn't an issue in go-github-ratelimit, it was related to the testing and the fact I was stopping at breakpoints.