Skip to content

Rate limit doesn't promise it will always be a positive integer #16683

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

Closed
1 task done
jsoref opened this issue Mar 30, 2022 · 2 comments
Closed
1 task done

Rate limit doesn't promise it will always be a positive integer #16683

jsoref opened this issue Mar 30, 2022 · 2 comments
Labels
content This issue or pull request belongs to the Docs Content team rest Content related to rest - overview. waiting for review Issue/PR is waiting for a writer's review

Comments

@jsoref
Copy link
Contributor

jsoref commented Mar 30, 2022

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/rest/guides/best-practices-for-integrators#dealing-with-secondary-rate-limits

What part(s) of the article would you like to see updated?

The current text (emphasis added) says:

When you have been limited, use the Retry-After response header to slow down. The value of the Retry-After header will always be an integer, representing the number of seconds you should wait before making requests again. For example, Retry-After: 30 means you should wait 30 seconds before sending more requests.

A number of other vendors (spotify/web-api#542, WebexCommunity/WebexPythonSDK#52) have at times sent retry-after: 0 when they in fact meant retry-after: 1.

In adding some code to deal w/ that case, I decided to check the documentation, and the documentation here doesn't promise that the server won't send retry-after: -10 or retry-after: 0.

I'm glad it's promising not to send the float 1.5, the string tomorrow, or the date-time 2038-01-19T03:14:08Z.

  • If it will always be > 0, it'd be nice if the documentation either promised that the value would be a positive integer, or a natural number.
  • If it might be 0, it'd be nice if the documentation specified whole number and then explained what one should do when the value is 0.
  • If it could be negative, the documentation should explain what one should do when that happens.

Additional information

I'm aware that this is a best practices guide.

These are really the only pages I can find that generally discuss the subject:

Of these pages, only Best practices for integrators actually talks about retry-after.

This repository used to have (non-user-facing) code that handled 429: 704f699

And it currently has:

  • This comment (which would trigger a PR from my tooling for this line, but, I'm not sure if that'd be particularly welcome):

    // Note that linkinator somtimes returns 429 and 503 errors for links that are not actually

  • And this, which definitely isn't user facing:

    const retryStatusCodes = [429, 503, 'Invalid']

It'd be great if a user facing page explicitly mentioned 429. (There's one REST API that mentions 429, but I don't know why #16682.)

@jsoref jsoref added the content This issue or pull request belongs to the Docs Content team label Mar 30, 2022
@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Mar 30, 2022
@ramyaparimi ramyaparimi added waiting for review Issue/PR is waiting for a writer's review rest Content related to rest - overview. and removed triage Do not begin working on this issue until triaged by the team labels Mar 30, 2022
@ramyaparimi
Copy link
Contributor

@jsoref
Thanks so much for opening an issue! I'll triage this for the team to review 👀

@skedwards88
Copy link
Contributor

Thanks for this issue. I discussed this with the API team. It's generally safe to assume that it will be a non-rational number with a value of 0 or greater. However, we don't plan to officially document this. Instead, users should write defensive code in the case that the return value does not meed the expected format (e.g. due to a system error on our part).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content This issue or pull request belongs to the Docs Content team rest Content related to rest - overview. waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

No branches or pull requests

3 participants