fix(error-reporting): silence search-query parse 400s (CLI-FA)#1151
Merged
Conversation
When a user passes invalid `--query` syntax (e.g. `is:403`, `lastSeen:>=-24h`, an empty `status:`), the Sentry search endpoint returns a 400 whose detail begins with "Error parsing search query: ...". classifySilenced captures all 400s by default (treating them as CLI-constructed malformed requests), so these user query-syntax mistakes were reported as crash reports — making CLI-FA one of the highest-volume issues (~450 users across issue/explore/trace list). Add a narrow exception: a 400 whose server detail reports an unparseable search query is a user input error, not a CLI bug. The API already returns an actionable message, so silence it (preserving volume via the cli.error.silenced metric). All other 400s remain captured. The match is a substring check on the server's own parser message, which is robust to the CLI re-wrapping list errors (enrichIssueListError prepends the original detail).
Contributor
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Contributor
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 5131 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.46% 81.47% +0.01%
==========================================
Files 397 397 —
Lines 27686 27694 +8
Branches 17972 17983 +11
==========================================
+ Hits 22555 22563 +8
- Misses 5131 5131 —
- Partials 1861 1861 —Generated by Codecov Action |
…erApiError Addresses Seer review: the upgrade-nudge logic (isUserError → getErrorUpdateNotification) and span-attribution logic (isUserApiError) only treated status > 400 as user errors, so a silenced search-query parse 400 would still trigger a misleading "Upgrading may resolve this" banner and be mis-attributed. Move isSearchQueryParseError into errors.ts as the single source of truth and use it from all three 4xx classifiers (classifySilenced, isUserError, isUserApiError) so they agree on query-parse 400s.
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
1 participant
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
When a user passes invalid
--querysyntax, the Sentry search endpoint returnsa 400 whose detail begins with
"Error parsing search query: ...". Examplesseen in production:
is:403→ invalid status value of '403'lastSeen:>=-24h→ Invalid date: >=-24h. Expected +/-durationstatus:→ Empty string after 'status:'issue.id:EVA-BCH→ Invalid numberclassifySilencedcaptures all 400s by default (treating them asCLI-constructed malformed requests), so these user query-syntax mistakes were
reported as crash reports — making CLI-FA
one of the highest-volume issues: ~450 users, still firing on 0.38.0 across
issue list,explore, andtrace list.Fix
Add a narrow exception in
classifySilenced: a 400 whose server detailreports an unparseable search query (
Error parsing search query) is a userinput error, not a CLI bug. The API already returns an actionable message, so
silence it and preserve volume via the
cli.error.silencedmetric. All other400s remain captured (the "400 = CLI bug" default is unchanged).
states it could not parse the user-supplied query, which a CLI-constructed
bad request would not produce.
(
enrichIssueListErrorprepends the original server detail before addinghints).
Test plan
isSearchQueryParseErrorhelper (exported for testing).api_query_error; wrapped issue-list 400 (detailprepended) →
api_query_error; non-query 400 → still captured (null);422with the marker → not treated as a query 400 (status must be 400).vitest run test/lib/error-reporting.test.ts→ 80 passedvitest run test/lib/telemetry.test.ts(classifySilenced consumer) → 107 passedbiome checkclean on changed filesFixes CLI-FA