From 108d76e6a8ada171ef508618f655d6a5210b90f1 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:48:51 +0000 Subject: [PATCH 1/2] fix(cli): Silence user-supplied search query syntax errors and validate failure_rate --- packages/cli/src/lib/error-reporting.ts | 39 +++++++++++++++- packages/cli/src/lib/errors.ts | 10 ++++- packages/cli/src/types/dashboard.ts | 1 + packages/cli/test/commands/issue/list.test.ts | 2 +- packages/cli/test/lib/error-reporting.test.ts | 44 +++++++++++++++++++ packages/cli/test/lib/errors.test.ts | 2 +- packages/cli/test/types/dashboard.test.ts | 10 +++++ 7 files changed, 104 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/lib/error-reporting.ts b/packages/cli/src/lib/error-reporting.ts index fc8be2699..f76a1c4f6 100644 --- a/packages/cli/src/lib/error-reporting.ts +++ b/packages/cli/src/lib/error-reporting.ts @@ -53,6 +53,7 @@ import { SeerError, TimeoutError, UpgradeError, + USER_QUERY_SYNTAX_FIELD, ValidationError, WizardError, } from "./errors.js"; @@ -74,6 +75,7 @@ type SilenceReason = | "process_exit" | "user_validation" | "user_input_error" + | "user_query_syntax" | "seer_unavailable_self_hosted"; /** @@ -136,6 +138,19 @@ export function classifySilenced(error: unknown): SilenceReason | null { if (error instanceof ValidationError && error.field === "directory") { return "user_input_error"; } + // The Sentry server rejected a search query the user typed (e.g. + // `issue.category:csp`, `status:401`). toSearchQueryError only uses this + // field when the user actually supplied `--query`, so it is a user input + // mistake that the CLI has no way to fix. The user already sees the + // server's detail plus a syntax hint. Queries the CLI builds itself still + // come through as raw 400 ApiErrors and stay captured. Volume stays visible + // through the `cli.error.silenced` metric and a structured log (CLI-1CZ). + if ( + error instanceof ValidationError && + error.field === USER_QUERY_SYNTAX_FIELD + ) { + return "user_query_syntax"; + } // A ResolutionError means the user provided a value (event ID, project slug, // etc.) that was looked up but not found. This is pure user-input noise, not // a CLI bug — the user sees a clear "not found" message (CLI-RP). @@ -206,6 +221,19 @@ function recordSilencedError(error: unknown, reason: SilenceReason): void { // Logger may not be initialized — ignore. } } + + // Structured log for rejected user queries — keeps the server's parse + // detail searchable in Sentry Logs now that these are not issues. + if (reason === "user_query_syntax" && error instanceof Error) { + try { + Sentry.logger.info("cli.query_syntax_silenced", { + detail: redactCredentialText(error.message).split("\n", 1)[0] ?? "", + }); + } catch (logError) { + // Logger may not be initialized; never block error handling. + void logError; + } + } } // --------------------------------------------------------------------------- @@ -319,7 +347,16 @@ function deriveErrorKind(error: Error): string | undefined { // (e.g. validateHexId throws with no `field`, so using field would // collapse every unfielded ValidationError into one group). if (error instanceof ValidationError) { - return error.field ?? extractMessagePrefix(error.message); + if (error.field === undefined) { + return extractMessagePrefix(error.message); + } + // "query" is shared by unrelated validators (aggregate names, empty alert + // queries, client-side query sanitizing). Keying on the field alone put + // all of them in one issue (CLI-1CZ), so add the message prefix. + if (error.field === "query") { + return `query: ${extractMessagePrefix(error.message)}`; + } + return error.field; } if (error instanceof ApiError) { return String(error.status); diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index 171b3e676..ff549e31a 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -854,6 +854,14 @@ export function isSearchQueryParseError(error: ApiError): boolean { export const SEARCH_QUERY_HELP = "Check your --query syntax (Sentry search reference: https://docs.sentry.io/concepts/search/)"; +/** + * `ValidationError.field` used when the Sentry server rejects a search query + * the user typed. It differs from the generic `"query"` field so error + * reporting can tell a user's own query mistake apart from a CLI-side query + * validation defect (CLI-1CZ). + */ +export const USER_QUERY_SYNTAX_FIELD = "query.user_syntax"; + /** * Convert a Sentry search-query parse failure into a {@link ValidationError} * when the user actually supplied the query; otherwise return the error @@ -899,7 +907,7 @@ export function toSearchQueryError( for (const suggestion of [SEARCH_QUERY_HELP, ...extraSuggestions]) { lines.push(` • ${suggestion}`); } - return new ValidationError(lines.join("\n"), "query"); + return new ValidationError(lines.join("\n"), USER_QUERY_SYNTAX_FIELD); } return error; } diff --git a/packages/cli/src/types/dashboard.ts b/packages/cli/src/types/dashboard.ts index 5e73981e3..34912b80a 100644 --- a/packages/cli/src/types/dashboard.ts +++ b/packages/cli/src/types/dashboard.ts @@ -270,6 +270,7 @@ export const SPAN_AGGREGATE_FUNCTIONS = [ "any", "min", "max", + "failure_rate", ] as const; export type SpanAggregateFunction = (typeof SPAN_AGGREGATE_FUNCTIONS)[number]; diff --git a/packages/cli/test/commands/issue/list.test.ts b/packages/cli/test/commands/issue/list.test.ts index 66ed0aaf8..2f1c41443 100644 --- a/packages/cli/test/commands/issue/list.test.ts +++ b/packages/cli/test/commands/issue/list.test.ts @@ -383,7 +383,7 @@ describe("issue list: error propagation", () => { expect.unreachable("Should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ValidationError); - expect((error as ValidationError).field).toBe("query"); + expect((error as ValidationError).field).toBe("query.user_syntax"); expect((error as Error).message).toContain("Error parsing search query"); } }); diff --git a/packages/cli/test/lib/error-reporting.test.ts b/packages/cli/test/lib/error-reporting.test.ts index a673192e7..3f397333b 100644 --- a/packages/cli/test/lib/error-reporting.test.ts +++ b/packages/cli/test/lib/error-reporting.test.ts @@ -33,6 +33,7 @@ import { OutputError, ResolutionError, SeerError, + USER_QUERY_SYNTAX_FIELD, ValidationError, WizardError, } from "../../src/lib/errors.js"; @@ -355,6 +356,25 @@ describe("classifySilenced", () => { ).toBe("user_input_error"); }); + test("silences a user's server-rejected --query (CLI-1CZ)", () => { + expect( + classifySilenced( + new ValidationError( + "Error parsing search query: Invalid category value of 'csp'", + USER_QUERY_SYNTAX_FIELD, + ), + ), + ).toBe("user_query_syntax"); + }); + + test("does NOT silence a CLI-side ValidationError with field 'query'", () => { + expect( + classifySilenced( + new ValidationError('Unknown aggregate function "bogus".', "query"), + ), + ).toBeNull(); + }); + test("silences ResolutionError (user provided a value that wasn't found)", () => { expect( classifySilenced( @@ -559,6 +579,30 @@ describe("reportCliError integration", () => { return { tags, contexts }; } + test("records a user query syntax error as a silenced metric, not an issue", () => { + reportCliError( + new ValidationError( + "Error parsing search query: Invalid category value of 'csp'", + USER_QUERY_SYNTAX_FIELD, + ), + ); + expect(captureSpy).not.toHaveBeenCalled(); + expect(metricSpy).toHaveBeenCalledWith( + "cli.error.silenced", + 1, + expect.objectContaining({ + attributes: expect.objectContaining({ reason: "user_query_syntax" }), + }), + ); + }); + + test("groups 'query' ValidationErrors by message prefix (CLI-1CZ)", () => { + const { tags } = capturedScopeTags( + new ValidationError('Unknown aggregate function "bogus".', "query"), + ); + expect(tags["cli_error.kind"]).toBe("query: Unknown aggregate function"); + }); + test("captures ContextError (no longer silenced) so its volume stays visible", () => { reportCliError( new ContextError("Organization and project", "sentry org view "), diff --git a/packages/cli/test/lib/errors.test.ts b/packages/cli/test/lib/errors.test.ts index 376893d58..0e6e31d5a 100644 --- a/packages/cli/test/lib/errors.test.ts +++ b/packages/cli/test/lib/errors.test.ts @@ -650,7 +650,7 @@ describe("toSearchQueryError", () => { test("converts a parse 400 to a ValidationError when the user supplied --query", () => { const result = toSearchQueryError(parseError(), "is:403"); expect(result).toBeInstanceOf(ValidationError); - expect((result as ValidationError).field).toBe("query"); + expect((result as ValidationError).field).toBe("query.user_syntax"); }); test("includes the server detail and a search-syntax suggestion in the message", () => { diff --git a/packages/cli/test/types/dashboard.test.ts b/packages/cli/test/types/dashboard.test.ts index a74e447e4..2547c07a7 100644 --- a/packages/cli/test/types/dashboard.test.ts +++ b/packages/cli/test/types/dashboard.test.ts @@ -134,6 +134,10 @@ describe("SPAN_AGGREGATE_FUNCTIONS", () => { expect(SPAN_AGGREGATE_FUNCTIONS).not.toContain("spm"); }); + test("contains failure_rate (CLI-1CZ)", () => { + expect(SPAN_AGGREGATE_FUNCTIONS).toContain("failure_rate"); + }); + test("valibot schema validates known functions", () => { expect(safeParse(SpanAggregateFunctionSchema, "count").success).toBe(true); expect(safeParse(SpanAggregateFunctionSchema, "p95").success).toBe(true); @@ -967,6 +971,12 @@ describe("mapWidgetTypeToDataset", () => { }); describe("validateAggregateNames", () => { + test("accepts failure_rate() for the spans dataset (CLI-1CZ)", () => { + expect(() => + validateAggregateNames(["count()", "failure_rate()"], "spans"), + ).not.toThrow(); + }); + test("accepts comma-separated tracemetrics aggregates", () => { expect(() => validateAggregateNames( From 4e342f98959b4ec38bd0190d530340eed065b0af Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:14:19 +0000 Subject: [PATCH 2/2] fix(cli): log query-syntax log emission failures instead of voiding them Replace the `void logError` catch, which sidesteps the no-silent-catch rule, with a log.debug call as required by packages/cli/AGENTS.md. --- packages/cli/src/lib/error-reporting.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/lib/error-reporting.ts b/packages/cli/src/lib/error-reporting.ts index f76a1c4f6..4bc164c62 100644 --- a/packages/cli/src/lib/error-reporting.ts +++ b/packages/cli/src/lib/error-reporting.ts @@ -57,6 +57,7 @@ import { ValidationError, WizardError, } from "./errors.js"; +import { logger } from "./logger.js"; import { isSaaS } from "./sentry-web-urls.js"; // --------------------------------------------------------------------------- @@ -231,7 +232,9 @@ function recordSilencedError(error: unknown, reason: SilenceReason): void { }); } catch (logError) { // Logger may not be initialized; never block error handling. - void logError; + logger + .withTag("error-reporting") + .debug("Failed to emit cli.query_syntax_silenced log", logError); } } }