fix(github): wait out primary rate limits and drop per-repo empty-check call#93
Conversation
…-check call
When analyzing a large GitHub org, repositories processed after the account's
primary hourly API rate limit was exhausted were silently skipped: every
remaining repo failed the empty-check with a 403 and was counted as
"not analyzed" (e.g. 2,636 of 7,219 repos dropped in one reported run, all at
the same second the 10k/hr limit was hit).
Two root causes, both fixed here:
1. Only the secondary ("abuse") rate limit was handled; the primary hourly
limit surfaces as *github.RateLimitError and was never waited out, so the
tool neither paused for the reset nor retried — it dropped the repo. Now the
go-github client context carries SleepUntilPrimaryRateLimitResetWhenRateLimited,
so primary limits are transparently slept-through and retried at both the
pre-emptive short-circuit and live-403 paths. Secondary-limit handling is
made robust (errors.As) and bounded by a retry cap.
2. reposIfEmpty made an extra ListCommits call per repo purely to detect empty
repos — doubling API-quota burn and adding the failing call above. Emptiness
is now read from repo.GetSize() (already returned by the listing API),
matching the existing check in processRepositoryBranches. This removes the
call, the error-drop path, and the dead function.
Verified with go vet, go build ./..., the getgithub package test suite, and gofmt.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Follow-up to the size-based empty check. GitHub computes repository size asynchronously, so a repo that DOES contain commits can transiently report size 0 (right after creation/import, or on GHES where size recalculation lags). Treating size 0 as unconditionally empty would silently drop such repos — the same symptom this branch set out to remove, via a different trigger (raised by review on PR #93). repoIsEmpty now treats size 0 as a *candidate* only: repos with size > 0 skip straight to analysis (zero-cost fast path, unchanged), while size-0 repos are confirmed with a single ListCommits(PerPage:1) guarded by the primary rate-limit sleep and secondary-limit retry. If the check is inconclusive, the repo is treated as non-empty and analyzed rather than dropped. Applied at all three call sites (GetReposGithub, GetGithubLanguages, processRepositoryBranches), and skipped-as-empty is now logged at INFO for operator auditability. Net API cost stays far below the original (which called ListCommits for every repo): the confirm call runs only for the small set of size-0 repos. Note: the review's specific suggestion — repo.GetPushedAt().IsZero() — was tested and rejected: a freshly created truly-empty repo also reports a non-null pushed_at, so it does not distinguish empty from non-empty. Verified live against the GitHub API: a fresh repo with one commit (size 0) is now analyzed, while a truly empty repo is still detected as empty. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es retry Raises new-code coverage on PR #93 (was 22.7%, gate needs 80%). Adds unit tests using the existing httptest fake-GHES pattern: - secondaryRateLimitPause: nil / unrelated / abuse-with-RetryAfter / abuse-without-RetryAfter (default) / wrapped-error cases (100%). - withRateLimitSleep: asserts the primary rate-limit sleep flag is set on ctx. - repoIsEmpty: size>0 fast path (no API call), and the size-0 confirm branch — has-commits, no-commits, "Git Repository is empty." (409), and inconclusive error → treated as non-empty (87.5%). - getAllBranches: success, secondary rate-limit retry-then-succeed, and non-rate-limit error propagation (94.1%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Code Review ✅ Approved 1 resolved / 1 findingsImplements transparent primary rate limit handling and replaces redundant API calls with size-based repository checks, resolving the empty-check misclassification and data loss issues. ✅ 1 resolved✅ Edge Case: Size-based empty check may misclassify non-empty repos as empty
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
1 participant
Problem
When analyzing a large GitHub organization, many repositories that should have been counted were silently dropped. In one reported run against a GitHub Enterprise Server org, 7,219 repos were discovered but only 4,559 analyzed — 2,636 skipped, every one logged as:
All 2,636 failures occurred in the same second — the moment the account's primary hourly API rate limit was exhausted. From that instant golc raced through the rest of the repo list, marking each as "not analyzed", instead of waiting ~28 min for the reset.
Root causes (both fixed here)
Primary rate limit was never handled. The code only caught the secondary/"abuse" limit (
*github.AbuseRateLimitError). The primary hourly limit surfaces as*github.RateLimitError— including go-github's pre-emptive short-circuit ("…not making remote request") — which was never waited out. On that errorreposIfEmptyreturned an error and the caller didnotAnalyzedCount++; continue, permanently dropping the repo.An extra
ListCommitscall per repo.reposIfEmptymade a dedicated API call just to detect empty repos — doubling quota burn (~2.2 calls/repo observed) and being the exact call that failed above.Changes
SleepUntilPrimaryRateLimitResetWhenRateLimited, so the client transparently sleeps until the reset and retries — at both the pre-emptive short-circuit and live-403 paths. Applied at all three context origins ingetgithub.go.errors.As(instead of a bare type assertion) and bounded by a retry cap (maxSecondaryRateLimitRetries).reposIfEmpty'sListCommitscall withrepo.GetSize() == 0, read from data already returned by the listing API — matching the existing check inprocessRepositoryBranches. Removes the extra call, the error-drop path, and the now-dead function. This roughly halves API usage per repo, so a fixed quota reaches ~2× the repositories.Net effect: a transient rate limit no longer causes permanent data loss, and the tool burns far less quota to begin with. Works identically for GitHub Cloud and Enterprise Server (driven by GitHub's standard rate-limit reset semantics).
Testing
go vet ./pkg/devops/getgithub/— cleango build ./...— cleango test ./pkg/devops/getgithub/— pass (obsoletereposIfEmptypanic test replaced with size-based empty-check tests)gofmt— clean🤖 Generated with Claude Code