Add retries for brew API failures#48306
Conversation
The Homebrew formulae API (served by GitHub Pages) intermittently returns 5xx errors, which previously aborted the entire ingestion run. This change adds smart retry logic with exponential backoff for transient failures (network errors, 5xx/429 responses) while preserving immediate failure for permanent errors (404, other 4xx). Implementation: - Created transientErr wrapper type to distinguish retryable from permanent failures - Wrapped fetchCask HTTP logic in retry.Do with configurable interval and max attempts - Added comprehensive tests verifying recovery after transient errors, exhaustion of attempts, and non-retrying of 404s
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48306 +/- ##
========================================
Coverage 67.42% 67.43%
========================================
Files 3665 3667 +2
Lines 231967 232579 +612
Branches 12304 12292 -12
========================================
+ Hits 156405 156840 +435
- Misses 61527 61637 +110
- Partials 14035 14102 +67
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:
|
![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 improves the reliability of the Homebrew maintained-apps ingester by adding retry logic around Homebrew formulae API fetches, so intermittent GitHub Pages 5xx/429 failures don’t abort an entire ingestion run.
Changes:
- Added configurable retry/backoff behavior to
brewIngester.fetchCaskfor transient HTTP/network failures. - Introduced a
transientErrwrapper + retry error filter to distinguish retryable vs permanent failures (e.g., 404). - Added tests covering transient recovery, attempt exhaustion, and non-retrying of 404s.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ee/maintained-apps/ingesters/homebrew/ingester.go | Adds retry/backoff with transient vs permanent error classification for Homebrew API fetches. |
| ee/maintained-apps/ingesters/homebrew/ingester_test.go | Adds unit tests validating retry behavior for transient and permanent failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Homebrew ingester now retries cask metadata requests with configurable interval and attempt limits. It treats 404 as a permanent missing-app error, retries selected transient failures, truncates error response bodies, and adds tests for transient recovery, retry exhaustion, and no-retry 404 handling. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ee/maintained-apps/ingesters/homebrew/ingester.go`:
- Around line 282-294: In the homebrew ingester retry path, transport/read
failures are always wrapped as transientErr even when the caller’s context has
already been canceled, which can cause needless retry/backoff loops. Update the
retry handling in i.client.Do and io.ReadAll error paths (and the similar block
around the later apply site) to check ctx.Err() before returning transientErr,
and if the context is done, return the context-cancellation error directly so
the retry filter stops immediately. Keep the transient retry behavior only for
genuine network/read interruptions while the context is still active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90ee5309-5426-45b5-a90f-0598468ece7d
📒 Files selected for processing (2)
ee/maintained-apps/ingesters/homebrew/ingester.goee/maintained-apps/ingesters/homebrew/ingester_test.go
The Homebrew formulae API (served by GitHub Pages) intermittently returns 5xx errors, which previously aborted the entire ingestion run. This change adds smart retry logic with exponential backoff for transient failures (network errors, 5xx/429 responses) while preserving immediate failure for permanent errors (404, other 4xx).
Implementation:
Summary by CodeRabbit