Retry transient GitHub errors in winget ingester#48775
Conversation
Replace direct GitHub Contents API calls for installer/locale manifest files with raw.githubusercontent.com fetches (CDN-backed, not subject to gitmon scheduling limits). Add exponential-backoff retry logic for transient 429/5xx errors on both the raw HTTP fetches and the directory listing API calls. Add tests covering retry behavior, max-attempt exhaustion, and 404 handling.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48775 +/- ##
=======================================
Coverage 68.02% 68.02%
=======================================
Files 3681 3682 +1
Lines 233961 234013 +52
Branches 12416 12416
=======================================
+ Hits 159146 159196 +50
- Misses 60498 60500 +2
Partials 14317 14317
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
raw.githubusercontent.com rate-limits unauthenticated requests per IP, which shared GitHub Actions runner IPs exhaust constantly. Send the existing PAT on raw fetches, and when raw is still throttled after retries, fall back to the contents API (the two are rate-limited independently). After 3 consecutive raw failures, skip raw for the rest of the run instead of burning a backoff cycle per file.
Two layers: - The winget ingester no longer trusts a 404 from raw.githubusercontent alone before falling through to an older version directory; the contents API must confirm the manifest is really missing. Raw is CDN-backed and a stale/spurious 404 would otherwise silently walk ingestion backwards to an older version. - cmd/maintained-apps now compares each ingested version against the version already published in the app's output manifest and refuses to write a regression (all platforms/ingesters). The app is skipped with a loud error while the rest of the run proceeds, so an upstream data hiccup can never produce a downgrade PR.
…k machinery
Reverts the retry/raw-fallback approach and replaces it with the minimal
change on top of the original code:
GitHub intermittently throttles access to the very hot winget-pkgs repo
("429 gitmon refuses to schedule us" during file-server congestion).
When ingesting an app hits a rate limit, skip it and move on: its
published manifest stays at the current version and it gets picked up
again on the next scheduled run. Other errors still fail the run.
The version-directory walk now only falls through to an older version
dir on a genuine 404; before, a 429 there was treated as "not found"
and could silently ingest an older version.
The same winget-pkgs file-server congestion that produces 429s also surfaces as 504s. Treat any 5xx like a rate limit: skip the app and let the next scheduled run pick it up.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Winget ingester’s error handling to better distinguish “missing manifest” (404) from transient GitHub/API failures, and adds tests around version fallback behavior.
Changes:
- Skip ingesting individual apps when a transient GitHub/API error is detected (instead of failing the entire run).
- Only fall back to older Winget version directories when the installer manifest fetch is a true 404; otherwise fail the app.
- Add a new test server + test cases to validate the version-walk behavior (404 fallthrough vs 429/504 failure).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ee/maintained-apps/ingesters/winget/ingester.go | Adds transient GitHub error classification, skips transient failures per-app, and tightens version fallback to 404-only. |
| ee/maintained-apps/ingesters/winget/ingester_test.go | Adds fixtures/tests to ensure 404 falls through to older versions, while 429/504 fails and is recognized as transient. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return | ||
| } | ||
| str := string(bytes) | ||
| content := &github.RepositoryContent{Name: new("Foo"), Content: &str} |
| case "/repos/microsoft/winget-pkgs/contents/manifests/f/Foo": | ||
| content := []github.RepositoryContent{ | ||
| {Name: new("2.0"), Type: new("dir")}, | ||
| {Name: new("1.0"), Type: new("dir")}, | ||
| } |
| // skip throttled apps; they'll be retried on the next scheduled run | ||
| if isTransientGitHubError(err) { | ||
| skippedApps++ | ||
| logger.WarnContext(ctx, "skipping app: GitHub rate-limited its ingestion; it will be retried on the next scheduled run", | ||
| "name", input.Name, "err", err) |
| if skippedApps > 0 { | ||
| logger.WarnContext(ctx, "some winget apps were skipped due to GitHub rate limiting", "count", skippedApps) | ||
| } |
| // isTransientGitHubError reports whether err is GitHub load-shedding (rate limits, 429s, 5xx). | ||
| func isTransientGitHubError(err error) bool { | ||
| if _, ok := errors.AsType[*github.RateLimitError](err); ok { | ||
| return true | ||
| } | ||
| if _, ok := errors.AsType[*github.AbuseRateLimitError](err); ok { | ||
| return true | ||
| } | ||
| if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response != nil { | ||
| return ghErr.Response.StatusCode == http.StatusTooManyRequests || ghErr.Response.StatusCode >= 500 | ||
| } | ||
| return false |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe winget ingester now classifies certain GitHub API errors (rate limit, abuse rate limit, HTTP 429, and HTTP 5xx) as transient. During bulk ingestion, apps that fail with transient errors are skipped and logged rather than aborting the entire run, with an aggregate warning at completion. Within single-app ingestion, installer manifest lookup failures now fall through to the next version directory only on HTTP 404; other errors are returned. A new test helper simulates a two-version package server, and a new test verifies fallback and error-classification behavior for 404, 429, and 504 responses. Changes
Related PRs: None identified. Suggested labels: Suggested reviewers: None identified. 🐰 A winget wobble, rate-limit blues, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This pull request improves the reliability and efficiency of the Winget ingester by introducing robust retry logic for fetching manifest files and directory contents, switching to CDN-backed raw file downloads, and updating tests to cover these changes. The main focus is on handling transient errors (like rate limits and server errors) gracefully, preventing ingestion failures due to temporary issues with GitHub's API or file servers.
Reliability improvements for manifest fetching:
getRawManifestFilemethod to fetch manifest files directly fromraw.githubusercontent.com(or a testable override), avoiding GitHub API rate limits and using CDN-backed downloads. This method implements retry logic for transient HTTP errors (e.g., 429, 5xx), with exponential backoff, and returns a specific error for missing files. (ee/maintained-apps/ingesters/winget/ingester.go)getRepoDirContentsmethod to list repository directories via the GitHub API with retry logic for transient errors, improving resilience against API throttling. (ee/maintained-apps/ingesters/winget/ingester.go)Ingestion logic updates:
ingestOneto use the new retry-enabled methods for both directory listing and manifest file fetching, ensuring that only true missing files are skipped and transient errors cause a controlled failure, not silent downgrades. (ee/maintained-apps/ingesters/winget/ingester.go) [1] [2]Test enhancements:
getRawManifestFileandgetRepoDirContentsretry behavior. (ee/maintained-apps/ingesters/winget/ingester_test.go) [1] [2]ee/maintained-apps/ingesters/winget/ingester_test.go)Dependency and setup changes:
io,net/http,net/url,time, andgithub.com/fleetdm/fleet/v4/pkg/retry) and updated struct initialization to support the new fields. (ee/maintained-apps/ingesters/winget/ingester.go,ee/maintained-apps/ingesters/winget/ingester_test.go) [1] [2] [3]These changes make the Winget ingestion process more robust against transient infrastructure issues and provide better test coverage for error handling and retry logic.
Summary by CodeRabbit
Bug Fixes
Tests