From 5f8bb6976f51068ca22cf490a961ed056740d560 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:38:00 +0000 Subject: [PATCH 1/2] fix(cli): Treat Events search syntax 400s as --query mistakes The Events endpoint returns event_search.py parser failures unwrapped ("Invalid quote at ...", "Parse error at ...", '"is:" queries are not supported ...', "timestamp: Invalid date: ..."). Only the issue-search wrapper "Error parsing search query" was recognized, so a typo in a user's explore/span/log/trace --query surfaced as a raw 400 and was reported to Sentry as a CLI bug (CLI-10). Recognize the grammar-level parser messages too, so toSearchQueryError turns them into a query ValidationError when the user supplied --query. Co-Authored-By: Miguel Betegon --- packages/cli/src/lib/errors.ts | 41 +++++++++++++++++----- packages/cli/test/commands/explore.test.ts | 41 ++++++++++++++++++++++ packages/cli/test/lib/errors.test.ts | 25 +++++++++++++ 3 files changed, 99 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index 909286f91..e00f2ef64 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -862,28 +862,53 @@ export function getExitCode(error: unknown): number { } /** - * Sentry's search-query parser emits a 400 whose detail begins with this phrase - * when the user's `--query` cannot be parsed (e.g. `is:403`, `lastSeen:>=-24h`, - * an empty `status:`). The server is telling us the user-supplied query is - * malformed — a user input error, not the CLI constructing a bad request. + * Issue search wraps search-query parser failures in this phrase + * (e.g. `is:403`, `lastSeen:>=-24h`, an empty `status:`). The server is telling + * us the user-supplied query is malformed — a user input error, not the CLI + * constructing a bad request. */ const SEARCH_QUERY_PARSE_MARKER = "Error parsing search query"; +/** + * The Events endpoint (explore, spans, logs, traces) returns the same parser + * failures unwrapped, as raised by `sentry/api/event_search.py`. Only + * grammar-level messages are listed: they describe text that could not be + * parsed, which the CLI's own `key:value` scope terms never produce. + */ +const EVENTS_SEARCH_PARSE_ERRORS: readonly RegExp[] = [ + /^Parse error at '/, + /^Invalid quote at '/, + /^Search terms cannot end with an escape character/, + /^Unexpected escape character: /, + /^Lists should not have empty values/, + /^Empty string after '/, + /^"is:" queries are not supported in this search/, + /^Invalid value for "is" search/, + /^"in" syntax invalid for "is" search/, + /^Boolean statements containing "OR" or "AND" are not supported/, + /^Invalid format for "has" search/, + /^\S+: Invalid (?:date|boolean|number): /, +]; + /** * Whether an {@link ApiError} is a Sentry search-query parse failure (HTTP 400 * whose detail reports an unparseable query). The CLI preserves the server's * detail when re-wrapping list errors (`enrichIssueListError` in - * `commands/issue/list.ts` prepends the original detail), so a substring match - * is robust to that wrapping. + * `commands/issue/list.ts` prepends the original detail), so the issue-search + * marker is matched as a substring. * * Shared by the three 4xx classifiers so they agree on these 400s: * `classifySilenced` (Sentry capture), {@link isUserError} (upgrade nudge), and * `isUserApiError` (span attribution). */ export function isSearchQueryParseError(error: ApiError): boolean { + const detail = error.detail; + if (error.status !== 400 || !detail) { + return false; + } return ( - error.status === 400 && - (error.detail?.includes(SEARCH_QUERY_PARSE_MARKER) ?? false) + detail.includes(SEARCH_QUERY_PARSE_MARKER) || + EVENTS_SEARCH_PARSE_ERRORS.some((pattern) => pattern.test(detail)) ); } diff --git a/packages/cli/test/commands/explore.test.ts b/packages/cli/test/commands/explore.test.ts index a742a83f1..e4c887c92 100644 --- a/packages/cli/test/commands/explore.test.ts +++ b/packages/cli/test/commands/explore.test.ts @@ -1348,6 +1348,47 @@ describe("sentry explore", () => { }); }); + describe("search syntax errors", () => { + const detail = + "Invalid quote at '*\"POST': quotes must enclose text or be escaped."; + + test("reports an unparseable --query as a query ValidationError", async () => { + resolveTargetSpy.mockResolvedValue({ org: "test-org" }); + queryEventsSpy.mockRejectedValue( + new ApiError("Bad request", 400, detail), + ); + const { context } = createContext(); + + const error = await func + .call( + context, + { + ...DEFAULT_FLAGS, + dataset: "spans", + query: 'span.description:*"POST', + }, + "test-org/", + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ValidationError); + expect(error).toMatchObject({ field: "query" }); + expect((error as Error).message).toContain(detail); + expect((error as Error).message).toContain("Check your --query syntax"); + }); + + test("keeps the ApiError when the CLI built the query", async () => { + resolveTargetSpy.mockResolvedValue({ org: "test-org" }); + const original = new ApiError("Bad request", 400, detail); + queryEventsSpy.mockRejectedValue(original); + const { context } = createContext(); + + await expect( + func.call(context, { ...DEFAULT_FLAGS, dataset: "spans" }, "test-org/"), + ).rejects.toBe(original); + }); + }); + describe("metrics dataset validation", () => { test("rejects standard aggregates on metrics dataset", async () => { resolveTargetSpy.mockResolvedValue({ org: "test-org" }); diff --git a/packages/cli/test/lib/errors.test.ts b/packages/cli/test/lib/errors.test.ts index 6806cc158..4d481cd69 100644 --- a/packages/cli/test/lib/errors.test.ts +++ b/packages/cli/test/lib/errors.test.ts @@ -624,6 +624,31 @@ describe("isSearchQueryParseError", () => { ); }); + test.each([ + "Parse error at '\n774ed' (column 12). This is commonly caused by unmatched parentheses. Enclose any text in double quotes.", + "Invalid quote at '*\"POST': quotes must enclose text or be escaped.", + "Search terms cannot end with an escape character", + "Empty string after 'release:'", + '"is:" queries are not supported in this search.', + "Invalid value for \"is\" search, valid values are ['archived', 'unresolved']", + "timestamp: Invalid date: >-15m. Expected +/-duration (e.g. +1h) or ISO 8601-like (e.g. 2026-11-03T10:00:38.16).", + ])("true for an unwrapped Events search parser detail: %s", (detail) => { + expect(isSearchQueryParseError(new ApiError("bad", 400, detail))).toBe( + true, + ); + }); + + test.each([ + "orderby must also be in the selected columns or groupby", + "Invalid query. Project(s) web do not exist or are not actively selected.", + "p75: column argument invalid: title is not a numeric column", + "Unknown function p10", + ])("false for a non-parser Events 400: %s", (detail) => { + expect(isSearchQueryParseError(new ApiError("bad", 400, detail))).toBe( + false, + ); + }); + test("false for a 400 without a query parse detail", () => { expect( isSearchQueryParseError(new ApiError("bad", 400, "Other failure")), From 30294e8b6c047c4d353024bad2c3bf107c3a3bc2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:30:01 +0000 Subject: [PATCH 2/2] fix(cli): Detect Events search parse 400s by their code Replace the list of parser message patterns with the "invalid_search_query" code the Events endpoint now returns next to "detail" (getsentry/sentry#129957). ApiError keeps the error body's code; its optional enriched403 flag moves into an options object to make room. Refs CLI-10 --- packages/cli/src/commands/alert/list-utils.ts | 2 +- packages/cli/src/commands/issue/list.ts | 12 ++--- packages/cli/src/lib/api/infrastructure.ts | 12 ++++- packages/cli/src/lib/errors.ts | 52 +++++++++---------- packages/cli/test/commands/explore.test.ts | 34 +++++++----- .../cli/test/lib/api-client.coverage.test.ts | 28 ++++++++++ .../cli/test/lib/api/infrastructure.test.ts | 17 ++++++ packages/cli/test/lib/errors.test.ts | 33 +++++------- .../init/tools/create-sentry-project.test.ts | 2 +- 9 files changed, 120 insertions(+), 72 deletions(-) diff --git a/packages/cli/src/commands/alert/list-utils.ts b/packages/cli/src/commands/alert/list-utils.ts index c98254ea4..43812c8a7 100644 --- a/packages/cli/src/commands/alert/list-utils.ts +++ b/packages/cli/src/commands/alert/list-utils.ts @@ -46,7 +46,7 @@ export function throwAlertListFetchFailure( error.status, error.detail, error.endpoint, - error.enriched403, + { enriched403: error.enriched403 }, ); } diff --git a/packages/cli/src/commands/issue/list.ts b/packages/cli/src/commands/issue/list.ts index 576d176ca..9920cce9b 100644 --- a/packages/cli/src/commands/issue/list.ts +++ b/packages/cli/src/commands/issue/list.ts @@ -1123,13 +1123,9 @@ function enrichIssueListError( const detail = error.enriched403 ? appendProjectMembershipHint(error.detail) : build403Detail(error.detail); - throw new ApiError( - error.message, - error.status, - detail, - error.endpoint, - true, - ); + throw new ApiError(error.message, error.status, detail, error.endpoint, { + enriched403: true, + }); } } throw error; @@ -1341,7 +1337,7 @@ async function handleResolvedTargets( first.status, detail, first.endpoint, - first.enriched403 || first.status === 403, + { enriched403: first.enriched403 || first.status === 403 }, ); } diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 5f5d0704f..eb643dc26 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -222,6 +222,14 @@ export type ApiRequestOptions = { idempotent?: boolean; }; +/** The string `code` of a JSON error body, e.g. `invalid_search_query`. */ +function readErrorCode(body: unknown): string | undefined { + if (body === null || typeof body !== "object" || !("code" in body)) { + return undefined; + } + return typeof body.code === "string" ? body.code : undefined; +} + /** * Throw an ApiError from a failed @sentry/api SDK response. * @@ -268,7 +276,7 @@ export function throwApiError( status, enrichDetail(status, detail, hasUsableDetail), undefined, - is403, + { enriched403: is403, code: readErrorCode(error) }, ); } @@ -751,7 +759,7 @@ async function throwRawApiError( response.status, enrichDetail(response.status, detail, detail !== undefined), endpoint, - is403, + { enriched403: is403, code: readErrorCode(body.json) }, ); } diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index e00f2ef64..c78eb9cc3 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -176,12 +176,19 @@ export class HostScopeError extends CliError { * @param status - HTTP status code * @param detail - Detailed error message from API response * @param endpoint - API endpoint that failed + * @param options - `enriched403` and `code`, documented on the fields below */ export class ApiError extends CliError { readonly status: number; readonly detail?: string; readonly endpoint?: string; + /** + * Machine-readable `code` from the error body, when the server sent one + * (e.g. `invalid_search_query`). + */ + readonly code?: string; + /** * Set by centralized 403 enrichment in `infrastructure.ts`. * Command-layer code can check this to avoid double-enriching @@ -194,7 +201,10 @@ export class ApiError extends CliError { status: number, detail?: string, endpoint?: string, - enriched403 = false, + { + enriched403 = false, + code, + }: { enriched403?: boolean; code?: string } = {}, ) { super(message, EXIT.API); this.name = "ApiError"; @@ -202,6 +212,7 @@ export class ApiError extends CliError { this.detail = detail; this.endpoint = endpoint; this.enriched403 = enriched403; + this.code = code; } override format(): string { @@ -870,45 +881,30 @@ export function getExitCode(error: unknown): number { const SEARCH_QUERY_PARSE_MARKER = "Error parsing search query"; /** - * The Events endpoint (explore, spans, logs, traces) returns the same parser - * failures unwrapped, as raised by `sentry/api/event_search.py`. Only - * grammar-level messages are listed: they describe text that could not be - * parsed, which the CLI's own `key:value` scope terms never produce. + * `code` the Events endpoint (explore, spans, logs, traces) sends next to + * `detail` when its search-query parser rejects the `query` parameter + * (getsentry/sentry#129957). */ -const EVENTS_SEARCH_PARSE_ERRORS: readonly RegExp[] = [ - /^Parse error at '/, - /^Invalid quote at '/, - /^Search terms cannot end with an escape character/, - /^Unexpected escape character: /, - /^Lists should not have empty values/, - /^Empty string after '/, - /^"is:" queries are not supported in this search/, - /^Invalid value for "is" search/, - /^"in" syntax invalid for "is" search/, - /^Boolean statements containing "OR" or "AND" are not supported/, - /^Invalid format for "has" search/, - /^\S+: Invalid (?:date|boolean|number): /, -]; +const SEARCH_QUERY_PARSE_CODE = "invalid_search_query"; /** * Whether an {@link ApiError} is a Sentry search-query parse failure (HTTP 400 - * whose detail reports an unparseable query). The CLI preserves the server's + * that reports an unparseable query). Events endpoints mark these with + * {@link SEARCH_QUERY_PARSE_CODE}; issue search only has the + * {@link SEARCH_QUERY_PARSE_MARKER} phrase. The CLI preserves the server's * detail when re-wrapping list errors (`enrichIssueListError` in - * `commands/issue/list.ts` prepends the original detail), so the issue-search - * marker is matched as a substring. + * `commands/issue/list.ts` prepends the original detail), so the marker is + * matched as a substring. * * Shared by the three 4xx classifiers so they agree on these 400s: * `classifySilenced` (Sentry capture), {@link isUserError} (upgrade nudge), and * `isUserApiError` (span attribution). */ export function isSearchQueryParseError(error: ApiError): boolean { - const detail = error.detail; - if (error.status !== 400 || !detail) { - return false; - } return ( - detail.includes(SEARCH_QUERY_PARSE_MARKER) || - EVENTS_SEARCH_PARSE_ERRORS.some((pattern) => pattern.test(detail)) + error.status === 400 && + (error.code === SEARCH_QUERY_PARSE_CODE || + (error.detail?.includes(SEARCH_QUERY_PARSE_MARKER) ?? false)) ); } diff --git a/packages/cli/test/commands/explore.test.ts b/packages/cli/test/commands/explore.test.ts index e4c887c92..b03fecddb 100644 --- a/packages/cli/test/commands/explore.test.ts +++ b/packages/cli/test/commands/explore.test.ts @@ -1351,24 +1351,21 @@ describe("sentry explore", () => { describe("search syntax errors", () => { const detail = "Invalid quote at '*\"POST': quotes must enclose text or be escaped."; + const flags = { + ...DEFAULT_FLAGS, + dataset: "spans", + query: 'span.description:*"POST', + }; + const parseError = (code?: string) => + new ApiError("Bad request", 400, detail, undefined, { code }); test("reports an unparseable --query as a query ValidationError", async () => { resolveTargetSpy.mockResolvedValue({ org: "test-org" }); - queryEventsSpy.mockRejectedValue( - new ApiError("Bad request", 400, detail), - ); + queryEventsSpy.mockRejectedValue(parseError("invalid_search_query")); const { context } = createContext(); const error = await func - .call( - context, - { - ...DEFAULT_FLAGS, - dataset: "spans", - query: 'span.description:*"POST', - }, - "test-org/", - ) + .call(context, flags, "test-org/") .catch((caught: unknown) => caught); expect(error).toBeInstanceOf(ValidationError); @@ -1379,7 +1376,7 @@ describe("sentry explore", () => { test("keeps the ApiError when the CLI built the query", async () => { resolveTargetSpy.mockResolvedValue({ org: "test-org" }); - const original = new ApiError("Bad request", 400, detail); + const original = parseError("invalid_search_query"); queryEventsSpy.mockRejectedValue(original); const { context } = createContext(); @@ -1387,6 +1384,17 @@ describe("sentry explore", () => { func.call(context, { ...DEFAULT_FLAGS, dataset: "spans" }, "test-org/"), ).rejects.toBe(original); }); + + test("keeps the ApiError when the 400 has no code", async () => { + resolveTargetSpy.mockResolvedValue({ org: "test-org" }); + const original = parseError(); + queryEventsSpy.mockRejectedValue(original); + const { context } = createContext(); + + await expect(func.call(context, flags, "test-org/")).rejects.toBe( + original, + ); + }); }); describe("metrics dataset validation", () => { diff --git a/packages/cli/test/lib/api-client.coverage.test.ts b/packages/cli/test/lib/api-client.coverage.test.ts index fd19c3f93..1c9f57fdf 100644 --- a/packages/cli/test/lib/api-client.coverage.test.ts +++ b/packages/cli/test/lib/api-client.coverage.test.ts @@ -1453,6 +1453,34 @@ describe("infrastructure.ts", () => { } }); + test("keeps the error body's code next to its detail", async () => { + globalThis.fetch = mockFetch( + async () => + new Response( + JSON.stringify({ + detail: "Invalid quote at '*\"POST'", + code: "invalid_search_query", + }), + { + status: 400, + statusText: "Bad Request", + headers: { "Content-Type": "application/json" }, + }, + ), + ); + + const error = await apiRequestToRegion( + "https://sentry.io", + "/test/", + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect(error).toMatchObject({ + detail: "Invalid quote at '*\"POST'", + code: "invalid_search_query", + }); + }); + test("JSON-stringifies error body without detail field", async () => { globalThis.fetch = mockFetch( async () => diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 57d8d0f0c..89f9390b5 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -205,6 +205,23 @@ describe("throwApiError", () => { } }); + test("HTTP error keeps the body's code", () => { + const mockResponse = new Response("", { + status: 400, + statusText: "Bad Request", + }); + + expect(() => + throwApiError( + { detail: "Invalid query syntax", code: "invalid_search_query" }, + mockResponse, + "Failed to list events", + ), + ).toThrow( + expect.objectContaining({ status: 400, code: "invalid_search_query" }), + ); + }); + test("HTTP error without detail uses stringified error", () => { const mockResponse = new Response("", { status: 500, diff --git a/packages/cli/test/lib/errors.test.ts b/packages/cli/test/lib/errors.test.ts index 4d481cd69..984bddf64 100644 --- a/packages/cli/test/lib/errors.test.ts +++ b/packages/cli/test/lib/errors.test.ts @@ -624,29 +624,24 @@ describe("isSearchQueryParseError", () => { ); }); - test.each([ - "Parse error at '\n774ed' (column 12). This is commonly caused by unmatched parentheses. Enclose any text in double quotes.", - "Invalid quote at '*\"POST': quotes must enclose text or be escaped.", - "Search terms cannot end with an escape character", - "Empty string after 'release:'", - '"is:" queries are not supported in this search.', - "Invalid value for \"is\" search, valid values are ['archived', 'unresolved']", - "timestamp: Invalid date: >-15m. Expected +/-duration (e.g. +1h) or ISO 8601-like (e.g. 2026-11-03T10:00:38.16).", - ])("true for an unwrapped Events search parser detail: %s", (detail) => { - expect(isSearchQueryParseError(new ApiError("bad", 400, detail))).toBe( - true, + test("true for a 400 with the invalid_search_query code", () => { + const error = new ApiError( + "bad", + 400, + "Invalid quote at '*\"POST': quotes must enclose text or be escaped.", + "/organizations/my-org/events/", + { code: "invalid_search_query" }, ); + expect(isSearchQueryParseError(error)).toBe(true); }); - test.each([ - "orderby must also be in the selected columns or groupby", - "Invalid query. Project(s) web do not exist or are not actively selected.", - "p75: column argument invalid: title is not a numeric column", - "Unknown function p10", - ])("false for a non-parser Events 400: %s", (detail) => { - expect(isSearchQueryParseError(new ApiError("bad", 400, detail))).toBe( - false, + test("false for a parser-looking Events 400 without the code", () => { + const error = new ApiError( + "bad", + 400, + "Invalid quote at '*\"POST': quotes must enclose text or be escaped.", ); + expect(isSearchQueryParseError(error)).toBe(false); }); test("false for a 400 without a query parse detail", () => { diff --git a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts index 5c921a938..91aee5cf7 100644 --- a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts +++ b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts @@ -299,7 +299,7 @@ describe("createSentryProject", () => { 403, "Your organization has disabled this feature for members.", undefined, - true, + { enriched403: true }, ), );