Skip to content

fix: detect secondary rate limit via response body marker#2267

Open
zhaomoran wants to merge 1 commit into
hub4j:mainfrom
zhaomoran:fix/secondary-rate-limit-body-detection
Open

fix: detect secondary rate limit via response body marker#2267
zhaomoran wants to merge 1 commit into
hub4j:mainfrom
zhaomoran:fix/secondary-rate-limit-body-detection

Conversation

@zhaomoran
Copy link
Copy Markdown

@zhaomoran zhaomoran commented May 26, 2026

Description

Adds a body-based fallback to GitHubAbuseLimitHandler#isError so that 403 responses carrying the "secondary rate" marker in the body are recognized as secondary rate limit errors, even when neither Retry-After nor gh-limited-by is present.

Fixes #2009

Why

GitHub does not always return Retry-After or gh-limited-by headers when a secondary rate limit is hit. As reported in #2009, endpoints like /search/issues can respond with a plain 403, no rate-limit headers, and only the body explains:

You have exceeded a secondary rate limit. Please wait a few minutes before you try again.

That response currently slips through GitHubAbuseLimitHandler#isError, so RetryRequestException is never thrown and the request just fails with a generic HttpException.

GitHub's official guidance on rate-limit errors explicitly states that Retry-After may be missing, and gh-limited-by is not part of the documented response headers at all, so we cannot rely solely on headers.

How

When the status is 403 and neither Retry-After nor gh-limited-by is set, read the response body and match \bsecondary rate\b (case-insensitive). The matching pattern mirrors octokit/plugin-throttling's /\bsecondary rate\b/i, which has been stable for years.

There is no extra I/O cost: GitHubConnectorResponse#bodyStream already buffers non-200 bodies into a byte array on the first read, so the body is read once and reused by both this check and any downstream handler that constructs an HttpException. This is the same pattern already used by STATUS_HTTP_BAD_REQUEST_OR_GREATER to detect Unicorn pages.

Tests

Added testHandler_Wait_Secondary_Limits_Body_Marker_Only, a WireMock scenario reproducing the #2009 case — a 403 with no Retry-After, no gh-limited-by, and the body marker — and asserts:

  • isError returns true (otherwise the handler is never called)
  • parseWaitTime falls back to DEFAULT_WAIT_MILLIS (61s, matching GitHub's "wait at least one minute" guidance)
  • The original request is retried and succeeds

All 10 tests in AbuseLimitHandlerTest pass locally.

Before submitting a PR:

  • 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.
  • 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 "-Dsurefire.argLine=--add-opens java.base/java.net=ALL-UNNAMED" locally. If this command doesn't succeed, your change will not pass CI.
  • Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

  • Fill in the "Description" above with clear summary of the changes. This includes:
    • If this PR fixes one or more issues, include "Fixes #" lines for each issue.
    • Provide links to relevant documentation on https://docs.github.com/en/rest where possible. If not including links, explain why not.
  • 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."
  • Enable "Allow edits from maintainers".

GitHub does not always return Retry-After or gh-limited-by headers
when a secondary rate limit is hit. In those cases the response is
a plain 403 whose body contains "You have exceeded a secondary rate
limit", which currently slips through GitHubAbuseLimitHandler#isError
and surfaces as a generic HttpException.

Fall back to matching the "secondary rate" marker in the response
body when neither header is present. The body for non-200 responses
is already buffered into memory by GitHubConnectorResponse, so this
adds no extra I/O for downstream handlers.

Fixes hub4j#2009
@zhaomoran zhaomoran force-pushed the fix/secondary-rate-limit-body-detection branch from 8614ced to 86f32e8 Compare May 26, 2026 05:46
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.31%. Comparing base (3622553) to head (86f32e8).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2267      +/-   ##
============================================
+ Coverage     85.30%   85.31%   +0.01%     
- Complexity     2553     2555       +2     
============================================
  Files           241      241              
  Lines          7540     7547       +7     
  Branches        396      397       +1     
============================================
+ Hits           6432     6439       +7     
  Misses          872      872              
  Partials        236      236              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitHubAbuseLimitHandler#isError fails to detect some secondary rate limit errors

1 participant