fix(telemetry): skip Sentry reporting for 4xx API errors#251
Merged
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Build
Bug Fixes 🐛Telemetry
Other
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
aa91c11 to
f3a6c61
Compare
Contributor
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 4094 uncovered lines. Files with missing lines (71)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 68.18% 68.51% +0.33%
==========================================
Files 108 109 +1
Lines 12978 13002 +24
Branches 0 0 —
==========================================
+ Hits 8849 8908 +59
- Misses 4129 4094 -35
- Partials 0 0 —Generated by Codecov Action |
4xx API errors (404 Not Found, 403 Forbidden, etc.) are user errors — wrong IDs, missing access — not CLI bugs. Recording them as exceptions creates noise in the Sentry issue stream without being actionable. Instead of capturing these as exceptions, record them as span attributes (api_error.status, api_error.message, api_error.detail) so they remain queryable in Discover for volume-spike detection without polluting the error feed. Extract isClientApiError() helper for classifying 4xx errors, following the same pattern as the existing isEpipeError() helper. Fixes CLI-5M Fixes CLI-46
f3a6c61 to
ef285ce
Compare
The 'with telemetry enabled' tests call initSentry(true) which sets global SDK state. Without cleanup, Sentry.isEnabled() returns true for all subsequent test files, breaking feedbackCommand's telemetry-disabled assertion.
BYK
added a commit
that referenced
this pull request
Feb 17, 2026
…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
BYK
added a commit
that referenced
this pull request
Feb 17, 2026
…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
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
captureExceptionandmarkSessionCrashedfor 4xxApiErrors (user errors like wrong IDs, no access)api_error.status,api_error.message,api_error.detail) for volume-spike detection in DiscoverisClientApiError()helper following the same pattern asisEpipeError()Motivation
CLI-5M and CLI-46 are
ApiError: 404 Not Foundfrom users passing non-existent issue/event IDs. These are expected operational errors, not bugs, but they were polluting the Sentry issue stream.Approach
Instead of dropping these errors entirely, they're recorded as span attributes so they remain queryable in Discover without creating issues:
A regression (e.g., broken API URL causing mass 404s) would show as a volume spike in span queries.
What's unchanged
ApiErrors → still reported as exceptionsAuthError("not_authenticated")→ still skipped (existing behavior)bin.tsfor user-facing formattingFixes CLI-5M, CLI-46