From 9156c20ebb95d13d39b87da9bc531fb374915ff4 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:32:15 +0000 Subject: [PATCH 1/4] fix(cli): Explain Explore aggregates rejected for their argument When Sentry rejects the function and argument of a user --field or --sort (a string attribute in p75, transaction.duration on the transactions dataset, a non-numeric errors column, an invalid count_if argument), raise a ValidationError with the server detail and a recovery hint instead of a raw 400 ApiError. Rejections of CLI-generated expressions stay ApiErrors. Refs CLI-10 Co-Authored-By: Miguel Betegon --- packages/cli/src/lib/explore-errors.ts | 64 ++++++++++++++++- packages/cli/test/commands/explore.test.ts | 80 +++++++++++++++++++++- 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/lib/explore-errors.ts b/packages/cli/src/lib/explore-errors.ts index 82b564332..65942d8aa 100644 --- a/packages/cli/src/lib/explore-errors.ts +++ b/packages/cli/src/lib/explore-errors.ts @@ -22,7 +22,11 @@ type ExploreErrorContext = { userSort: string | undefined; }; -/** Match narrow backend diagnostics without maintaining an aggregate catalog. */ +/** + * Match narrow backend diagnostics without maintaining an aggregate catalog. + * Beyond the specific corrections, a diagnostic that names this expression's + * own function and argument gets a generic hint. + */ function recoveryHint( detail: string, expression: string, @@ -56,10 +60,66 @@ function recoveryHint( if (detail === `Unknown function ${parsed.name}`) { return "Aggregate functions depend on the dataset. Check the function name and selected dataset; sentry explore --help includes usage examples."; } + + const rejected = rejectedArgument(detail); + if ( + !rejected || + rejected.name !== parsed.name || + !parsed.arguments.includes(rejected.argument) + ) { + return; + } + const { argument, reason } = rejected; + // `--dataset transactions` queries spans, which keep durations in span.duration. + if (dataset === "spans" && argument === "transaction.duration") { + return "Use span.duration instead of transaction.duration. Transactions are queried as spans, which store their duration in span.duration."; + } + const lookup = `sentry attribute list / ${argument} --dataset ${dataset}`; + if (reason === "type") { + return ATTRIBUTE_DATASETS.has(dataset) + ? `${parsed.name}() needs a numeric attribute, and ${argument} is not numeric on dataset "${dataset}". Reference a numeric custom attribute with its type, e.g. tags[${argument},number]; to find it, run: ${lookup} --type number` + : `${parsed.name}() needs a numeric column, and ${argument} is not numeric on dataset "${dataset}".`; + } + return ATTRIBUTE_DATASETS.has(dataset) + ? `${parsed.name}() does not accept ${argument} on dataset "${dataset}". Check the attribute key; to find it, run: ${lookup}` + : `${parsed.name}() does not accept ${argument} on dataset "${dataset}". Check the column name.`; +} + +/** Datasets whose custom attributes use `tags[name,type]` keys listed by `sentry attribute list`. */ +const ATTRIBUTE_DATASETS = new Set(["spans", "logs"]); + +/** Backend diagnostics that name the function and argument they rejected. */ +const REJECTED_ARGUMENT_PATTERNS: [RegExp, "type" | "invalid"][] = [ + // e.g. "duration is invalid for parameter 1 in p75. Its a string type field, ..." + [/^(?.+?) is invalid for parameter \d+ in (?\w+)\./, "type"], + // e.g. "min: column argument invalid: title is not a numeric column" + [ + /^(?\w+): column argument invalid: (?.+?) is not a numeric column/, + "type", + ], + // e.g. "avg: column argument invalid: memory is not a valid column" + [ + /^(?\w+): column argument invalid: (?.+?) is not a valid column/, + "invalid", + ], + // e.g. "severity is not a valid argument for count_if" + [/^(?.+?) is not a valid argument for (?\w+)/, "invalid"], +]; + +/** The function and argument a backend diagnostic rejected, when it names them. */ +function rejectedArgument( + detail: string, +): { name: string; argument: string; reason: "type" | "invalid" } | undefined { + for (const [pattern, reason] of REJECTED_ARGUMENT_PATTERNS) { + const groups = pattern.exec(detail)?.groups; + if (groups?.name && groups.argument) { + return { name: groups.name, argument: groups.argument, reason }; + } + } } /** - * Add a correction for a known rejected user expression, preserving the server + * Add a correction for a rejected user expression, preserving the server * detail. Unknown failures and errors caused by CLI-generated input are returned * unchanged so they retain their API error classification and diagnostics. */ diff --git a/packages/cli/test/commands/explore.test.ts b/packages/cli/test/commands/explore.test.ts index a742a83f1..32677a528 100644 --- a/packages/cli/test/commands/explore.test.ts +++ b/packages/cli/test/commands/explore.test.ts @@ -1279,6 +1279,75 @@ describe("sentry explore", () => { expect((error as Error).message).not.toContain("attribute list"); }); + test.each([ + [ + "a string attribute in a numeric aggregate", + { dataset: "spans", field: ["span.op", "p75(cart.total)"] }, + "cart.total is invalid for parameter 1 in p75. Its a string type field, but it must be one of these types: {'number'}", + '--field "p75(cart.total)"', + "tags[cart.total,number]; to find it, run: sentry attribute list / cart.total --dataset spans --type number", + ], + [ + "transaction.duration on transactions", + { + dataset: "transactions", + field: ["transaction", "sum(transaction.duration)"], + sort: "-sum(transaction.duration)", + }, + "transaction.duration is invalid for parameter 1 in sum. Its a string type field, but it must be one of these types: {'number', 'duration'}", + '--field "sum(transaction.duration)" for dataset "spans"', + "Use span.duration instead of transaction.duration.", + ], + [ + "a timestamp aggregate on spans", + { dataset: "spans", field: ["min(timestamp)"] }, + "timestamp is invalid for parameter 1 in min. Its a string type field, but it must be one of these types: {'number'}", + '--field "min(timestamp)"', + "min() needs a numeric attribute, and timestamp is not numeric", + ], + [ + "a non-numeric errors column", + { field: ["issue", "min(title)"] }, + "min: column argument invalid: title is not a numeric column", + '--field "min(title)" for dataset "errors"', + "min() needs a numeric column, and title is not numeric", + ], + [ + "an unknown errors column passed only through --sort", + { field: ["device", "count()"], sort: "-avg(device.memory)" }, + "avg: column argument invalid: device.memory is not a valid column", + '--sort "-avg(device.memory)"', + "Check the column name.", + ], + [ + "an invalid conditional-count argument", + { dataset: "logs", field: ["count_if(severity,equals,error)"] }, + "severity is not a valid argument for count_if", + '--field "count_if(severity,equals,error)"', + "sentry attribute list / severity --dataset logs", + ], + ])( + "attributes %s to the user's expression", + async (_label, flags, detail, rejected, hint) => { + resolveTargetSpy.mockResolvedValue({ org: "test-org" }); + queryEventsSpy.mockRejectedValue( + new ApiError("Bad request", 400, detail), + ); + const { context } = createContext(); + + const error = await func + .call(context, { ...DEFAULT_FLAGS, ...flags }, "test-org/") + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ValidationError); + expect((error as Error).message).toContain(detail); + expect((error as Error).message).toContain(rejected); + expect((error as Error).message).toContain(hint); + expect((error as Error).message).not.toContain("timestamp_precise"); + expect(isUserError(error)).toBe(true); + }, + ); + test.each([ ["default fields", {}, "Unknown function count"], [ @@ -1297,9 +1366,14 @@ describe("sentry explore", () => { "Unknown function p10", ], [ - "a spans backend without timestamp aggregates", - { dataset: "spans", field: ["min(timestamp)"] }, - "timestamp is invalid for parameter 1 in min. Its a string type field", + "a rejected argument the user did not pass", + { dataset: "spans", field: ["p75(span.duration)"] }, + "cart.total is invalid for parameter 1 in p75. Its a string type field", + ], + [ + "a rejected argument of a default field", + { query: "level:error" }, + "min: column argument invalid: title is not a numeric column", ], [ "an unknown rejection", 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 2/4] 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 3/4] 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 }, ), ); From 2e3c154e3f9a42fdbeb9ee6db606997b57ae7218 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:48:42 +0000 Subject: [PATCH 4/4] fix(cli): Read the rejected function argument from the 400 body Replace the detail-message patterns with the structured fields that getsentry/sentry#129954 adds to function-argument 400s: code invalid_function_argument plus function and argument. A 400 without the code stays an ApiError. --- packages/cli/src/lib/api/infrastructure.ts | 33 ++++-- packages/cli/src/lib/errors.ts | 19 ++- packages/cli/src/lib/explore-errors.ts | 67 ++++------- packages/cli/test/commands/explore.test.ts | 109 +++++++++++++----- .../cli/test/lib/api-client.coverage.test.ts | 29 +++++ .../cli/test/lib/api/infrastructure.test.ts | 24 ++++ 6 files changed, 200 insertions(+), 81 deletions(-) diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 056f9a4fb..039670c09 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -40,6 +40,7 @@ import { getEnv } from "../env.js"; import { ApiError, CliError, + type FunctionArgument, stringifyUnknown, TimeoutError, } from "../errors.js"; @@ -230,12 +231,30 @@ 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; +/** + * Structured fields of a JSON error body: its string `code` (e.g. + * `invalid_search_query`) and the rejected `function` and `argument` + * (sent with `invalid_function_argument`). + */ +function readErrorFields(body: unknown): { + code?: string; + functionArgument?: FunctionArgument; +} { + if (body === null || typeof body !== "object") { + return {}; } - return typeof body.code === "string" ? body.code : undefined; + const fields = body as Record; + const code = typeof fields.code === "string" ? fields.code : undefined; + if ( + typeof fields.function !== "string" || + typeof fields.argument !== "string" + ) { + return { code }; + } + return { + code, + functionArgument: { function: fields.function, argument: fields.argument }, + }; } const CONNECT_TIMEOUT_HINT = @@ -371,7 +390,7 @@ export function throwApiError( status, enrichDetail(status, detail, hasUsableDetail), undefined, - { enriched403: is403, code: readErrorCode(error) }, + { enriched403: is403, ...readErrorFields(error) }, ); } @@ -858,7 +877,7 @@ async function throwRawApiError( response.status, enrichDetail(response.status, detail, detail !== undefined), endpoint, - { enriched403: is403, code: readErrorCode(body.json) }, + { enriched403: is403, ...readErrorFields(body.json) }, ); } diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index c78eb9cc3..c2c2e25bb 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -169,6 +169,9 @@ export class HostScopeError extends CliError { } } +/** A function argument named by a Sentry 400 error body. */ +export type FunctionArgument = { function: string; argument: string }; + /** * API request errors from Sentry. * @@ -176,7 +179,7 @@ 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 + * @param options - Structured error fields, documented on the fields below */ export class ApiError extends CliError { readonly status: number; @@ -189,6 +192,12 @@ export class ApiError extends CliError { */ readonly code?: string; + /** + * Function and argument the server rejected, sent with code + * `invalid_function_argument` (e.g. `p75` and `cart.total`). + */ + readonly functionArgument?: FunctionArgument; + /** * Set by centralized 403 enrichment in `infrastructure.ts`. * Command-layer code can check this to avoid double-enriching @@ -204,7 +213,12 @@ export class ApiError extends CliError { { enriched403 = false, code, - }: { enriched403?: boolean; code?: string } = {}, + functionArgument, + }: { + enriched403?: boolean; + code?: string; + functionArgument?: FunctionArgument; + } = {}, ) { super(message, EXIT.API); this.name = "ApiError"; @@ -213,6 +227,7 @@ export class ApiError extends CliError { this.endpoint = endpoint; this.enriched403 = enriched403; this.code = code; + this.functionArgument = functionArgument; } override format(): string { diff --git a/packages/cli/src/lib/explore-errors.ts b/packages/cli/src/lib/explore-errors.ts index 65942d8aa..6846079d8 100644 --- a/packages/cli/src/lib/explore-errors.ts +++ b/packages/cli/src/lib/explore-errors.ts @@ -6,7 +6,7 @@ import { parseFunction } from "@sentry/toolkit-core/events-fields"; import { getSortField } from "@sentry/toolkit-core/events-sort"; -import { ApiError, validationError } from "./errors.js"; +import { ApiError, type FunctionArgument, validationError } from "./errors.js"; /** User-authored expressions and the request they contributed to. */ type ExploreErrorContext = { @@ -22,13 +22,17 @@ type ExploreErrorContext = { userSort: string | undefined; }; +/** `code` of a 400 that names the function argument it rejected (getsentry/sentry#129954). */ +const INVALID_FUNCTION_ARGUMENT_CODE = "invalid_function_argument"; + /** * Match narrow backend diagnostics without maintaining an aggregate catalog. - * Beyond the specific corrections, a diagnostic that names this expression's - * own function and argument gets a generic hint. + * Beyond the specific corrections, a 400 whose structured fields name this + * expression's own function and argument gets a generic hint. */ function recoveryHint( detail: string, + rejected: FunctionArgument | undefined, expression: string, dataset: string, ): string | undefined { @@ -61,63 +65,27 @@ function recoveryHint( return "Aggregate functions depend on the dataset. Check the function name and selected dataset; sentry explore --help includes usage examples."; } - const rejected = rejectedArgument(detail); if ( !rejected || - rejected.name !== parsed.name || + rejected.function !== parsed.name || !parsed.arguments.includes(rejected.argument) ) { return; } - const { argument, reason } = rejected; + const { argument } = rejected; // `--dataset transactions` queries spans, which keep durations in span.duration. if (dataset === "spans" && argument === "transaction.duration") { return "Use span.duration instead of transaction.duration. Transactions are queried as spans, which store their duration in span.duration."; } - const lookup = `sentry attribute list / ${argument} --dataset ${dataset}`; - if (reason === "type") { - return ATTRIBUTE_DATASETS.has(dataset) - ? `${parsed.name}() needs a numeric attribute, and ${argument} is not numeric on dataset "${dataset}". Reference a numeric custom attribute with its type, e.g. tags[${argument},number]; to find it, run: ${lookup} --type number` - : `${parsed.name}() needs a numeric column, and ${argument} is not numeric on dataset "${dataset}".`; - } + const problem = `${parsed.name}() does not accept ${argument} on dataset "${dataset}": it may not exist there, or may not have the type ${parsed.name}() needs.`; return ATTRIBUTE_DATASETS.has(dataset) - ? `${parsed.name}() does not accept ${argument} on dataset "${dataset}". Check the attribute key; to find it, run: ${lookup}` - : `${parsed.name}() does not accept ${argument} on dataset "${dataset}". Check the column name.`; + ? `${problem} Reference a numeric custom attribute with its type, e.g. tags[${argument},number]; to check the key and its type, run: sentry attribute list / ${argument} --dataset ${dataset}` + : `${problem} Check the column name and its type.`; } /** Datasets whose custom attributes use `tags[name,type]` keys listed by `sentry attribute list`. */ const ATTRIBUTE_DATASETS = new Set(["spans", "logs"]); -/** Backend diagnostics that name the function and argument they rejected. */ -const REJECTED_ARGUMENT_PATTERNS: [RegExp, "type" | "invalid"][] = [ - // e.g. "duration is invalid for parameter 1 in p75. Its a string type field, ..." - [/^(?.+?) is invalid for parameter \d+ in (?\w+)\./, "type"], - // e.g. "min: column argument invalid: title is not a numeric column" - [ - /^(?\w+): column argument invalid: (?.+?) is not a numeric column/, - "type", - ], - // e.g. "avg: column argument invalid: memory is not a valid column" - [ - /^(?\w+): column argument invalid: (?.+?) is not a valid column/, - "invalid", - ], - // e.g. "severity is not a valid argument for count_if" - [/^(?.+?) is not a valid argument for (?\w+)/, "invalid"], -]; - -/** The function and argument a backend diagnostic rejected, when it names them. */ -function rejectedArgument( - detail: string, -): { name: string; argument: string; reason: "type" | "invalid" } | undefined { - for (const [pattern, reason] of REJECTED_ARGUMENT_PATTERNS) { - const groups = pattern.exec(detail)?.groups; - if (groups?.name && groups.argument) { - return { name: groups.name, argument: groups.argument, reason }; - } - } -} - /** * Add a correction for a rejected user expression, preserving the server * detail. Unknown failures and errors caused by CLI-generated input are returned @@ -142,8 +110,17 @@ export function toExploreFieldError( }); } + const rejected = + error.code === INVALID_FUNCTION_ARGUMENT_CODE + ? error.functionArgument + : undefined; for (const { expression, value, flag } of expressions) { - const hint = recoveryHint(error.detail, expression, context.dataset); + const hint = recoveryHint( + error.detail, + rejected, + expression, + context.dataset, + ); if (hint) { return validationError( `Sentry rejected --${flag} "${value}" for dataset "${context.dataset}".\n\n${error.detail}`, diff --git a/packages/cli/test/commands/explore.test.ts b/packages/cli/test/commands/explore.test.ts index 175e9ca89..cc0188fc6 100644 --- a/packages/cli/test/commands/explore.test.ts +++ b/packages/cli/test/commands/explore.test.ts @@ -1365,13 +1365,34 @@ describe("sentry explore", () => { expect((error as Error).message).not.toContain("attribute list"); }); + const rejectedArgument = (fn: string, argument: string) => ({ + code: "invalid_function_argument", + functionArgument: { function: fn, argument }, + }); + const rejectedArgumentError = ( + detail: string, + fn: string, + argument: string, + ) => + new ApiError( + "Bad request", + 400, + detail, + undefined, + rejectedArgument(fn, argument), + ); + test.each([ [ "a string attribute in a numeric aggregate", { dataset: "spans", field: ["span.op", "p75(cart.total)"] }, - "cart.total is invalid for parameter 1 in p75. Its a string type field, but it must be one of these types: {'number'}", + rejectedArgumentError( + "cart.total is invalid for parameter 1 in p75. Its a string type field, but it must be one of these types: {'number'}", + "p75", + "cart.total", + ), '--field "p75(cart.total)"', - "tags[cart.total,number]; to find it, run: sentry attribute list / cart.total --dataset spans --type number", + "tags[cart.total,number]; to check the key and its type, run: sentry attribute list / cart.total --dataset spans", ], [ "transaction.duration on transactions", @@ -1380,45 +1401,63 @@ describe("sentry explore", () => { field: ["transaction", "sum(transaction.duration)"], sort: "-sum(transaction.duration)", }, - "transaction.duration is invalid for parameter 1 in sum. Its a string type field, but it must be one of these types: {'number', 'duration'}", + rejectedArgumentError( + "transaction.duration is invalid for parameter 1 in sum. Its a string type field, but it must be one of these types: {'number', 'duration'}", + "sum", + "transaction.duration", + ), '--field "sum(transaction.duration)" for dataset "spans"', "Use span.duration instead of transaction.duration.", ], [ "a timestamp aggregate on spans", { dataset: "spans", field: ["min(timestamp)"] }, - "timestamp is invalid for parameter 1 in min. Its a string type field, but it must be one of these types: {'number'}", + rejectedArgumentError( + "timestamp is invalid for parameter 1 in min. Its a string type field, but it must be one of these types: {'number'}", + "min", + "timestamp", + ), '--field "min(timestamp)"', - "min() needs a numeric attribute, and timestamp is not numeric", + 'min() does not accept timestamp on dataset "spans"', ], [ "a non-numeric errors column", { field: ["issue", "min(title)"] }, - "min: column argument invalid: title is not a numeric column", + rejectedArgumentError( + "min(title): column argument invalid: title is not a numeric column", + "min", + "title", + ), '--field "min(title)" for dataset "errors"', - "min() needs a numeric column, and title is not numeric", + "Check the column name and its type.", ], [ "an unknown errors column passed only through --sort", { field: ["device", "count()"], sort: "-avg(device.memory)" }, - "avg: column argument invalid: device.memory is not a valid column", + rejectedArgumentError( + "avg(device.memory): column argument invalid: device.memory is not a valid column", + "avg", + "device.memory", + ), '--sort "-avg(device.memory)"', - "Check the column name.", + 'avg() does not accept device.memory on dataset "errors"', ], [ "an invalid conditional-count argument", { dataset: "logs", field: ["count_if(severity,equals,error)"] }, - "severity is not a valid argument for count_if", + rejectedArgumentError( + "severity is not a valid argument for count_if", + "count_if", + "severity", + ), '--field "count_if(severity,equals,error)"', "sentry attribute list / severity --dataset logs", ], ])( "attributes %s to the user's expression", - async (_label, flags, detail, rejected, hint) => { + async (_label, flags, apiError, rejected, hint) => { resolveTargetSpy.mockResolvedValue({ org: "test-org" }); - queryEventsSpy.mockRejectedValue( - new ApiError("Bad request", 400, detail), - ); + queryEventsSpy.mockRejectedValue(apiError); const { context } = createContext(); const error = await func @@ -1426,7 +1465,7 @@ describe("sentry explore", () => { .catch((caught: unknown) => caught); expect(error).toBeInstanceOf(ValidationError); - expect((error as Error).message).toContain(detail); + expect((error as Error).message).toContain(apiError.detail); expect((error as Error).message).toContain(rejected); expect((error as Error).message).toContain(hint); expect((error as Error).message).not.toContain("timestamp_precise"); @@ -1455,11 +1494,18 @@ describe("sentry explore", () => { "a rejected argument the user did not pass", { dataset: "spans", field: ["p75(span.duration)"] }, "cart.total is invalid for parameter 1 in p75. Its a string type field", + rejectedArgument("p75", "cart.total"), ], [ "a rejected argument of a default field", { query: "level:error" }, - "min: column argument invalid: title is not a numeric column", + "min(title): column argument invalid: title is not a numeric column", + rejectedArgument("min", "title"), + ], + [ + "a rejected argument from a server that sends no code", + { dataset: "spans", field: ["p75(cart.total)"] }, + "cart.total is invalid for parameter 1 in p75. Its a string type field", ], [ "an unknown rejection", @@ -1475,18 +1521,27 @@ describe("sentry explore", () => { }, "Unknown function sum", ], - ])("preserves the API error for %s", async (_label, flags, detail) => { - resolveTargetSpy.mockResolvedValue({ org: "test-org" }); - const original = new ApiError("Bad request", 400, detail); - queryEventsSpy.mockRejectedValue(original); - const { context } = createContext(); + ])( + "preserves the API error for %s", + async (_label, flags, detail, fields?) => { + resolveTargetSpy.mockResolvedValue({ org: "test-org" }); + const original = new ApiError( + "Bad request", + 400, + detail, + undefined, + fields, + ); + queryEventsSpy.mockRejectedValue(original); + const { context } = createContext(); - await expect( - func.call(context, { ...DEFAULT_FLAGS, ...flags }, "test-org/"), - ).rejects.toBe(original); - expect(isUserError(original)).toBe(false); - expect(classifySilenced(original)).toBeNull(); - }); + await expect( + func.call(context, { ...DEFAULT_FLAGS, ...flags }, "test-org/"), + ).rejects.toBe(original); + expect(isUserError(original)).toBe(false); + expect(classifySilenced(original)).toBeNull(); + }, + ); test("preserves non-400 errors even when their detail mentions a user field", async () => { resolveTargetSpy.mockResolvedValue({ org: "test-org" }); diff --git a/packages/cli/test/lib/api-client.coverage.test.ts b/packages/cli/test/lib/api-client.coverage.test.ts index e384bb440..eda59d83a 100644 --- a/packages/cli/test/lib/api-client.coverage.test.ts +++ b/packages/cli/test/lib/api-client.coverage.test.ts @@ -1502,6 +1502,35 @@ describe("infrastructure.ts", () => { }); }); + test("keeps the function and argument a 400 body rejected", async () => { + globalThis.fetch = mockFetch( + async () => + new Response( + JSON.stringify({ + detail: "cart.total is invalid for parameter 1 in p75.", + code: "invalid_function_argument", + function: "p75", + argument: "cart.total", + }), + { + status: 400, + statusText: "Bad Request", + headers: { "Content-Type": "application/json" }, + }, + ), + ); + + const error = await apiRequestToRegion( + "https://sentry.io", + "/test/", + ).catch((caught: unknown) => caught); + + expect(error).toMatchObject({ + code: "invalid_function_argument", + functionArgument: { function: "p75", argument: "cart.total" }, + }); + }); + 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 dc29c4818..4ba46a5ae 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -323,6 +323,30 @@ describe("throwApiError", () => { ); }); + test("HTTP error keeps the function and argument the body rejected", () => { + const mockResponse = new Response("", { + status: 400, + statusText: "Bad Request", + }); + + expect(() => + throwApiError( + { + detail: "cart.total is invalid for parameter 1 in p75.", + code: "invalid_function_argument", + function: "p75", + argument: "cart.total", + }, + mockResponse, + "Failed to list events", + ), + ).toThrow( + expect.objectContaining({ + functionArgument: { function: "p75", argument: "cart.total" }, + }), + ); + }); + test("HTTP error without detail uses stringified error", () => { const mockResponse = new Response("", { status: 500,