Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 92.87%. Comparing base (2b8c7fa) to head (960ce81).
Report is 43 commits behind head on master.

❗ Current head 960ce81 differs from pull request most recent head ea82df3. Consider uploading reports for the commit ea82df3 to get more accurate results

@@            Coverage Diff             @@
##           master    #3117      +/-   ##
==========================================
- Coverage   97.72%   92.87%   -4.85%     
==========================================
  Files         153      170      +17     
  Lines       13390    11445    -1945     
==========================================
- Hits        13085    10630    -2455     
- Misses        215      724     +509     
- Partials       90       91       +1     

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

It would also be nice to add one or more unit tests that would increase the code coverage for the new code.

Thank you, @erezrokah ! LGTM.

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

Thanks for the quick review and feedback, I also added a 1 second buffer to the sleep time to avoid any edge cases

Hi @gmlewis I'm wondering if I should update the PR to take into account context cancellation, by changing sleepUntilResetWithBuffer to

timer := time.NewTimer(time.Until(reset) + buffer)
select {
case <-ctx.Done():
	if !timer.Stop() {
	<-timer.C
	}
	return nil, ctx.Err()
case <-timer.C:
}

Ah, that's an excellent idea, @erezrokah - I think you are right that this should be added.

Ah, that's an excellent idea, @erezrokah - I think you are right that this should be added.

Added 22eba4c. In one case I had to wrap the context error in a rate limit error. I can also refactor the code a bit to have checkRateLimitBeforeDo return an error instead of *RateLimitError, but thought it would be better to get some feedback first

Thank you, @be0x74a !
Merging.

Thanks for the review(s) @be0x74a and @gmlewis, when can we expect a new release to be tagged with this commit?

Thanks for the review(s) @be0x74a and @gmlewis, when can we expect a new release to be tagged with this commit?

I'm away from Internet this week so the earliest will be this Friday.