diff --git a/packages/cli/src/commands/alert/list-utils.ts b/packages/cli/src/commands/alert/list-utils.ts index 23a357aa4..b5dbd7b6e 100644 --- a/packages/cli/src/commands/alert/list-utils.ts +++ b/packages/cli/src/commands/alert/list-utils.ts @@ -49,7 +49,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 3c480361b..2a01252e7 100644 --- a/packages/cli/src/commands/issue/list.ts +++ b/packages/cli/src/commands/issue/list.ts @@ -1124,13 +1124,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; @@ -1349,7 +1345,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 2d0db4fc9..056f9a4fb 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -230,6 +230,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; +} + const CONNECT_TIMEOUT_HINT = "The connection timed out. Check network access and firewall rules on this machine."; @@ -363,7 +371,7 @@ export function throwApiError( status, enrichDetail(status, detail, hasUsableDetail), undefined, - is403, + { enriched403: is403, code: readErrorCode(error) }, ); } @@ -850,7 +858,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 909286f91..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 { @@ -862,19 +873,28 @@ 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"; +/** + * `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 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 a substring match - * is robust to that wrapping. + * `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 @@ -883,7 +903,8 @@ const SEARCH_QUERY_PARSE_MARKER = "Error parsing search query"; export function isSearchQueryParseError(error: ApiError): boolean { return ( error.status === 400 && - (error.detail?.includes(SEARCH_QUERY_PARSE_MARKER) ?? false) + (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 239422a87..bdecdcda3 100644 --- a/packages/cli/test/commands/explore.test.ts +++ b/packages/cli/test/commands/explore.test.ts @@ -1434,6 +1434,55 @@ 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(parseError("invalid_search_query")); + const { context } = createContext(); + + const error = await func + .call(context, flags, "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 = parseError("invalid_search_query"); + queryEventsSpy.mockRejectedValue(original); + const { context } = createContext(); + + await expect( + 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", () => { test("rejects standard aggregates on metrics dataset", 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 1cdcf9919..e384bb440 100644 --- a/packages/cli/test/lib/api-client.coverage.test.ts +++ b/packages/cli/test/lib/api-client.coverage.test.ts @@ -1474,6 +1474,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 c51e1831b..dc29c4818 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -306,6 +306,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 6806cc158..984bddf64 100644 --- a/packages/cli/test/lib/errors.test.ts +++ b/packages/cli/test/lib/errors.test.ts @@ -624,6 +624,26 @@ describe("isSearchQueryParseError", () => { ); }); + 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("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", () => { expect( isSearchQueryParseError(new ApiError("bad", 400, "Other failure")), 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 }, ), );