fix(errors): name the failing API from one place - #38
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 18 minutes Limit details: You’ve used all 2 included reviews currently available. Your 59 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. You can run this review on demand instead of waiting. On-demand reviews are free until September 18, 2026. After that, they cost $0.25 per reviewed file.
How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Included review availability: 1 review is currently available. Based on recent review activity, included reviews refill at 2 per hour. 📝 WalkthroughWalkthroughOrganization lookup HTTP failures now raise ChangesOrganization lookup error reporting
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change improves organization-discovery error messages and adds focused test coverage; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR #38 improved the org-resolution error by hand-rolling a message that named the failing API. That fixed one site and left the other 123 on the shared formatter, so the CLI grew a second error format. Move the idea into handle_http_error instead, and route the outliers through it: - handle_http_error names the API when the host isn't the public default. The host is configurable via AVR_HOST or hosts.json, so on a self-hosted or staging endpoint a bare status never said which API answered. On the default host it stays quiet, where the URL is noise on every error. - _fetch_user_orgs and get_verified_org_slug now use the shared formatter. Org resolution is usually the first authenticated request a command makes, so it is where a stale token surfaces: it was the one place that reported a raw 401 instead of the auth hint and exit 4 that scripts check for. - Repo resolution no longer renders httpx's own exception string (a two-line message ending in an MDN link) for non-404 failures. - The login flow keeps its own handling, since a 401 there means the browser session expired rather than "go run avr auth login", but its two handlers now name the host like the rest of that file already did. Tests pin both halves of the host rule, the 401 exit code at both migrated sites, and the absence of httpx internals in repo errors.
Mutation testing the previous commit found three changes that kept the suite green when reverted, so they were not actually pinned: - The 401 branch names the API in a sentence of its own, but the only 401 test used the default host, where that sentence is suppressed. Deleting the line changed nothing observable. Now covered from both sides: named on a custom host, absent on the default one. - get_verified_org_slug's move onto the shared formatter was never executed. Its existing tests cover a transport error and a non-JSON 200, neither of which is an HTTPStatusError. - The login flow's split between status and transport errors was never executed. Added a fixture that drives login() to the API-key exchange with no browser and no socket, following the pattern already used in test_auth_sso. All twelve mutants are now caught, including full reverts of each migrated call site.
Summary
The original fix improved one error message by hand-rolling a string that named the failing API. That left the other 123 HTTP error sites on the shared
handle_http_errorformatter, so the CLI would have grown a second error format. This moves the idea into the shared formatter instead.handle_http_errornow names the API when the host isn't the public default. The host is configurable throughAVR_HOSTorhosts.json, so against a self-hosted or staging endpoint a bare status never said which API answered. On the default host it stays quiet, where the URL is noise on every error.avr auth loginhint and exit code 4 that scripts check for.avr auth login". Its two handlers now name the host, matching what the rest of that file already did.Behaviour
Default host, unchanged:
Custom host:
Expired token during organization resolution, previously exit 1 with a raw status:
Exit code is now 4 at both migrated sites, matching every other API call.
Tests
Pin both halves of the host rule (named when custom, silent when default), the 401 exit code at both migrated call sites, and the absence of httpx internals in repository errors.
Verification
uv run --extra dev python -m pytest tests -q(798 passed)make lintgit diff --check