Resolves #1842. Resolves #1805.

Description

If a user uses this library to drive the GitHub API hard, there will quite often be failures caused by secondary rate limits being breached (#1842 and #1805). There’s been some discussion of headers on #1842, and also of pluggable 'handle the rate limit' modules, but I think there’s a more fundamental problem with the handling of rate limits. (The good news is this means there's a simpler solution, too.)

The AbuseLimitHandler checks for a rate limit violation by looking for a 403 header and some headers:  

@Override
boolean isError(@Nonnull GitHubConnectorResponse connectorResponse) {
    return isForbidden(connectorResponse) && hasRetryOrLimitHeader(connectorResponse);
}

HTTP_FORBIDDEN is a 403. The GitHub docs indicate that limits from GitHub are signalled with either 403 or TOO_MANY_REQUESTS responses (429). In my experience, I've only seen 429s.

What about headers?

The GitHub docs docs suggest that any headers are optional, and that in the absence of headers the code should just wait a minute (which, luckily, is what the current implementation already does):

If the retry-after response header is present, you should not retry your request until after that many seconds has elapsed. If the x-ratelimit-remaining header is 0, you should not retry your request until after the time, in UTC epoch seconds, specified by the x-ratelimit-reset header. Otherwise, wait for at least one minute before retrying. If your request continues to fail due to a secondary rate limit, wait for an exponentially increasing amount of time between retries, and throw an error after a specific number of retries.

Based on these docs, I think it's sufficient to count a 429 as a rate limit response, even if headers are missing. Semantically, a 429 is an indication of an abuse detection/rate limit scenario, and should be handled as such.

What about the rate limit handler?

GitHubRateLimitHandler also requires a 403 to 'count' a response as rate limited:

@Override
boolean isError(@NotNull GitHubConnectorResponse connectorResponse) throws IOException {
connectorResponse.statusCode() == HttpURLConnection.HTTP_FORBIDDEN
            && "0".equals(connectorResponse.header("X-RateLimit-Remaining"));

I didn't update this one, since the secondary rate limit checks are now correctly being handled on the abuse path with my changes. It might be worth updating it, or also even considering whether the distinction between the abuse and rate limit paths is so fuzzy that having both is just maintenance overhead. But that’s a different discussion.

Before submitting a PR:

[ X] Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
[ X] Add JavaDocs and other comments explaining the behavior.
[ ] When adding or updating methods that fetch entities, add @link JavaDoc entries to the relevant documentation on https://docs.github.com/en/rest .
[ ] Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
[ ] Run mvn -D enable-ci clean install site locally. If this command doesn't succeed, your change will not pass CI.
[ X] Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

[X] Fill in the "Description" above with clear summary of the changes. This includes:
- [X ] If this PR fixes one or more issues, include "Fixes #" lines for each issue.
- [ X] Provide links to relevant documentation on https://docs.github.com/en/rest where possible. If not including links, explain why not.
[X ] All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
[X] Enable "Allow edits from maintainers".

Files	Patch %	Lines
...va/org/kohsuke/github/GitHubAbuseLimitHandler.java	33.33%	0 Missing and 2 partials :warning:
...ain/java/org/kohsuke/github/AbuseLimitHandler.java	0.00%	1 Missing :warning:

hub4j / github-api

Accept 429 response codes as an indication of the secondary rate limit being exceeded #1895

Description

What about headers?

What about the rate limit handler?

Before submitting a PR:

When creating a PR:

Codecov Report