-
Notifications
You must be signed in to change notification settings - Fork 34
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
When retrying based on x-ratelimit-reset
, take local time and server time into account
#599
Comments
A mentee of mine ran into the problem using code that I haven't changed for a long time. It sure feels like something changed: Rutasd/hearted_contributions#1 (comment) I'll take care of the date sync but also add a few seconds extra buffer |
for reference, this is how we account for time skew in |
I also saw the identical |
I think we should totally add a second. Does not implement the fix described in this issue, but it's much simpler and I think we should do it. 👍🏼 |
I've seen the plugin handle the primary rate limit correctly, wait until the provided
x-ratelimit-reset
time, and then immediately hit the rate limit again. My guess is that local time and server time are out-of-sync. It's something we had to take into account for the timeouts of installation access tokens as well in https://github.com/octokit/auth-app.js.The text was updated successfully, but these errors were encountered: