Add and detect AbuseRateLimitError.#441
Add and detect AbuseRateLimitError.#441gmlewis merged 1 commit intogoogle:masterfrom sourcegraph:abuse-rate-limit-error
Conversation
|
Some notes on this. I'm implementing this based on GitHub documentation (and a clarifying/confirmation email I've sent to support@github.com). I've not had the opportunity to test it for real because I don't want to try to reproduce an abuse rate limit situation on purpose just for testing this PR. |
| // RetryAfter is provided with some abuse rate limit errors. If present, | ||
| // it is the amount of time that the client should wait before retrying. | ||
| // Otherwise, the client should try again later (after an unspecified amount of time). | ||
| RetryAfter *time.Duration |
There was a problem hiding this comment.
I initially tried to use a value rather than pointer:
RetryAfter time.DurationBut, I changed it to be a pointer to be able to differentiate between a missing Retry-After header (which means "retry later after an unspecified time") vs a present Retry-After header with value of 0 seconds. I have no evidence/proof that 0 is an impossible value, that's why I couldn't use it as a sentinel to mean "unspecified time". If 0 were to come up in reality, it means "retry right now", which is very close to "retry after 1 second", and very different from "retry after unspecified time".
| Response: errorResponse.Response, | ||
| Message: errorResponse.Message, | ||
| } | ||
| case r.StatusCode == http.StatusForbidden && errorResponse.DocumentationURL == "https://developer.github.com/v3#abuse-rate-limits": |
There was a problem hiding this comment.
The reason I matched based on the documentation_url value rather than a prefix of the message is because this seems more reliable.
I know there are multiple different types of abuse rate limit errors, and they have different message text. They may have the same prefix, but I don't know for sure what are all the possible messages. If the documentation_url value is "https://developer.github.com/v3#abuse-rate-limits" (note the "abuse" in the URL, it's not https://developer.github.com/v3/#rate-limiting), then it seems quite reasonable to expect this is an abuse rate limit error rather than something else.
If there are improvement suggestions, I'd be glad to hear them.
It's similar to RateLimitError, but it's a different type of rate limit error. It is documented at: https://developer.github.com/v3/#abuse-rate-limits Parse and include the Retry-After header value, if present. It is documented at: https://developer.github.com/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits According to GitHub support, its type is an integer value representing seconds. Helps #431.
|
I like it. Thank you, @shurcooL! |
2 participants
It's similar to
RateLimitError, but it's a different type of rate limit error. It is documented at:https://developer.github.com/v3/#abuse-rate-limits
Parse and include the
Retry-Afterheader value, if present. It is documented at:https://developer.github.com/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits
According to GitHub support, its type is an integer value representing seconds.
Helps #431.