fix(issue-list): propagate original errors instead of wrapping in plain Error#254
Merged
fix(issue-list): propagate original errors instead of wrapping in plain Error#254
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Build
Other
Bug Fixes 🐛Telemetry
Other
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ Patch coverage is 94.29%. Project has 3693 uncovered lines. Files with missing lines (70)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 71.21% 73.06% +1.85%
==========================================
Files 111 111 —
Lines 13764 13707 -57
Branches 0 0 —
==========================================
+ Hits 9801 10014 +213
- Misses 3963 3693 -270
- Partials 0 0 —Generated by Codecov Action |
…ailures When all project fetches fail, re-throw the original ApiError (with status, detail, endpoint) instead of a plain Error. This lets the telemetry layer in PR #251 correctly classify 4xx errors as client errors and suppress them from Sentry exceptions. Also: - Classify more HTTP status codes (404, 429, other 4xx) instead of lumping everything into "unknown" - Include partial failure info in JSON output when some projects fail - Warn on stderr about partial failures in human output - Add tests for error propagation and partial failure handling
bfc1739 to
027bf80
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the highest-volume production telemetry issue ("Failed to fetch issues from N project(s)" — CLI-3Q: 132 events, CLI-N: 81 events).
The root cause:
fetchIssuesForTarget()caught all errors and discarded the originalApiError, re-throwing a plainError. This meant telemetry always calledcaptureException(no suppression rule matched), and JSON output lost the HTTP status code.Changes
ApiErrorinstead of plainErrorwhen all project fetches fail, so the telemetry layer (isClientApiError()from fix(telemetry): skip Sentry reporting for 4xx API errors #251) correctly suppresses 4xx client errors{ issues, errors }with raw HTTP status codes; human output warns on stderrJSON error format
When some project fetches fail, JSON output includes error details with raw HTTP status codes (not string classifications):
{ "issues": [...], "errors": [ { "status": 400, "message": "Invalid query syntax" }, { "message": "Network timeout" } ] }statusis present only for API errors. Non-API errors (network, etc.) have onlymessage.Test plan
7 new tests in
test/commands/issue/list.test.ts:ApiErrorwith correct statusApiError.detailflows through{ issues, errors }withstatusfieldAll 1721 unit tests pass. Typecheck and lint clean.
Depends on #251 for full telemetry suppression of 4xx errors in production.