From 6b14659a7d01612173b929e7cb46f572940cf50c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 21 Aug 2026 14:33:32 +0000 Subject: [PATCH 1/2] fix(api): report empty error responses --- .lore.md | 3 + packages/cli/src/commands/api.ts | 162 +++++++++++++++--- packages/cli/src/lib/api/infrastructure.ts | 10 +- packages/cli/test/commands/api.test.ts | 119 +++++++++++++ packages/cli/test/e2e/api.test.ts | 59 ++++++- .../cli/test/lib/api/infrastructure.test.ts | 15 ++ packages/cli/test/mocks/routes.ts | 5 + 7 files changed, 349 insertions(+), 24 deletions(-) diff --git a/.lore.md b/.lore.md index 00c5b54b2c..53b0b609ce 100644 --- a/.lore.md +++ b/.lore.md @@ -4,6 +4,9 @@ ### Architecture + +* **API output binary and HTTP contracts**: Always preserve strict output-mode and binary-safety contracts. `sentry api` must preserve `rawApiRequest()` status text; empty textual non-2xx bodies must produce an HTTP status/request fallback, and JSON mode must expose `{status, statusText, body}` so empty success and error responses are distinguishable. Binary `Uint8Array` successes bypass formatters and remain raw; binary errors emit only status/content-type/byte-count summaries, never bytes or JSON coercion. Prompts must never block scripted runs or interleave with stdout JSON. + * **@sentry/symbolic 13.4.0 API surface: SourceBundleWriter for bundle-sources command**: \`@sentry/symbolic@13.4.0\` exports 4 classes: \`Archive\`, \`FileEntry\`, \`ObjectFile\`, \`SourceBundleWriter\`, plus \`SourceFileDescriptor\`. Key for CLI source-tier commands: \`SourceBundleWriter.writeObject(object: ObjectFile, object\_name: string, filter: Function, provider: Function): Uint8Array | undefined\` — callback-based; provider reads source content by path, filter selects files. \`bundle-sources\` is directly implementable (provider reads from disk). \`print-sources\` is BLOCKED — \`ObjectFile\` has no \`sourceFiles()\` enumeration method in 13.4.0 (only props: arch, codeId, debugId, fileFormat, hasDebugInfo, hasSources, hasSymbols, hasUnwindInfo, kind). \`SourceFileDescriptor\` has get/set props: contents, debugId, path, sourceMappingUrl, url, type. Confirmed by Dav1dde (Sebastian Zivota's colleague) on Jun 23 2026. diff --git a/packages/cli/src/commands/api.ts b/packages/cli/src/commands/api.ts index 908cd05abb..91912319b8 100644 --- a/packages/cli/src/commands/api.ts +++ b/packages/cli/src/commands/api.ts @@ -12,6 +12,7 @@ import type { SentryContext } from "../context.js"; import { buildSearchParams, rawApiRequest } from "../lib/api-client.js"; import { buildCommand } from "../lib/command.js"; import { OutputError, ValidationError } from "../lib/errors.js"; +import { filterFields } from "../lib/formatters/json.js"; import { CommandOutput } from "../lib/formatters/output.js"; import { validateEndpoint } from "../lib/input-validation.js"; import { logger } from "../lib/logger.js"; @@ -924,6 +925,117 @@ export function formatBinaryErrorBody( ); } +/** + * Format an empty textual error body with enough request context to diagnose + * a routing miss. Sentry can return an empty body for unmatched API routes. + * @internal Exported for testing + */ +export function formatEmptyErrorBody( + status: number, + statusText: string | undefined, + method: string | undefined, + endpoint: string | undefined +): string { + const statusLabel = [status, statusText].filter(Boolean).join(" "); + const request = method && endpoint ? ` — ${method} /api/0/${endpoint}` : ""; + return `HTTP ${statusLabel}${request}`; +} + +type ApiResponseOutputOptions = { + silent: boolean; + isTTY: boolean | undefined; + json?: boolean; + method?: string; + endpoint?: string; +}; + +type ApiResponseOutput = { + status: number; + statusText?: string; + headers: Headers; + body: unknown; +}; + +/** Throw the appropriate output error for a non-successful API response. */ +function throwApiResponseError( + response: ApiResponseOutput, + options: ApiResponseOutputOptions +): never { + const isBinary = response.body instanceof Uint8Array; + const errorBody = isBinary + ? formatBinaryErrorBody( + response.status, + response.headers, + response.body as Uint8Array + ) + : response.body; + + if (options.json) { + throw new OutputError({ + status: response.status, + statusText: response.statusText ?? "", + body: errorBody, + }); + } + + if (isBinary) { + throw new OutputError(errorBody); + } + if ( + response.body === null || + response.body === undefined || + (typeof response.body === "string" && response.body.trim() === "") + ) { + throw new OutputError( + formatEmptyErrorBody( + response.status, + response.statusText, + options.method, + options.endpoint + ) + ); + } + throw new OutputError(response.body); +} + +/** Add HTTP metadata to textual responses in JSON output mode. */ +function formatApiResponseOutput( + response: ApiResponseOutput, + output: unknown, + json: boolean +): unknown { + if (!json || output instanceof Uint8Array) { + return output; + } + return { + status: response.status, + statusText: response.statusText, + body: output, + }; +} + +/** Preserve API body field filtering inside the response envelope. */ +function formatApiResponseJson(data: unknown, fields?: string[]): unknown { + if ( + data === null || + typeof data !== "object" || + Array.isArray(data) || + !("status" in data) || + !("body" in data) + ) { + return data; + } + + const response = data as ApiResponseOutput; + return { + ...response, + body: + fields && fields.length > 0 + ? filterFields(response.body, fields) + : response.body, + }; +} + /** * Resolve the full URL that rawApiRequest would use for a request. * @@ -1129,8 +1241,14 @@ function logRequest( } /** Log incoming response details in `< ` curl-verbose style. */ -function logResponse(response: { status: number; headers: Headers }): void { - log.debug(`< HTTP ${response.status}`); +function logResponse(response: { + status: number; + statusText?: string; + headers: Headers; +}): void { + log.debug( + `< HTTP ${response.status}${response.statusText ? ` ${response.statusText}` : ""}` + ); response.headers.forEach((value, key) => { log.debug(`< ${key}: ${value}`); }); @@ -1143,7 +1261,8 @@ function logResponse(response: { status: number; headers: Headers }): void { * - silent: no body (OutputError(null) on error for exit code only) * - binary error: short status/content-type summary (never dump bytes) * - binary success: raw Uint8Array (the func decides TTY rendering/warning) - * - text/JSON: body as-is for the formatter path + * - text: body as-is for the formatter path + * - JSON: body wrapped with HTTP status metadata by the command * * Extracted from the command func to keep cognitive complexity under the lint threshold. * @@ -1151,12 +1270,16 @@ function logResponse(response: { status: number; headers: Headers }): void { * @throws {OutputError} on HTTP error statuses * @internal Exported for testing */ +/** Every status outside the 2xx range is an API error. */ +export function isApiErrorStatus(status: number): boolean { + return status < 200 || status >= 300; +} + export function resolveApiResponseOutput( - response: { status: number; headers: Headers; body: unknown }, - options: { silent: boolean; isTTY: boolean | undefined } + response: ApiResponseOutput, + options: ApiResponseOutputOptions ): unknown { - const isError = response.status >= 400; - const isBinary = response.body instanceof Uint8Array; + const isError = isApiErrorStatus(response.status); if (options.silent) { if (isError) { @@ -1166,16 +1289,7 @@ export function resolveApiResponseOutput( } if (isError) { - if (isBinary) { - throw new OutputError( - formatBinaryErrorBody( - response.status, - response.headers, - response.body as Uint8Array - ) - ); - } - throw new OutputError(response.body); + throwApiResponseError(response, options); } // Binary success: return raw bytes. The command func decides whether to @@ -1226,7 +1340,7 @@ export function resolveBinaryTtyOutput( } export const apiCommand = buildCommand({ - output: { human: formatApiResponse }, + output: { human: formatApiResponse, jsonTransform: formatApiResponseJson }, docs: { brief: "Make an authenticated API request", fullDescription: @@ -1383,7 +1497,7 @@ export const apiCommand = buildCommand({ headers, }); - const isError = response.status >= 400; + const isError = isApiErrorStatus(response.status); if (verbose) { logResponse(response); @@ -1403,6 +1517,9 @@ export const apiCommand = buildCommand({ const output = resolveApiResponseOutput(response, { silent: flags.silent, isTTY: this.stdout.isTTY, + json: flags.json, + method: flags.method, + endpoint: normalizedEndpoint, }); if (output === undefined) { return; @@ -1425,7 +1542,10 @@ export const apiCommand = buildCommand({ } // Binary Uint8Array bodies are written raw by renderCommandOutput (no - // formatter, no trailing newline). Text/JSON go through formatApiResponse. - return yield new CommandOutput(output); + // formatter, no trailing newline). Textual JSON responses expose the HTTP + // metadata so callers can distinguish an empty success from an error. + return yield new CommandOutput( + formatApiResponseOutput(response, output, flags.json) + ); }, }); diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index e4311ec713..4136f56cc1 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -718,13 +718,18 @@ export function isTextualContentType(contentType: string | null): boolean { * * @param endpoint - API endpoint path (e.g., "/organizations/") * @param options - Request options including method, body, params, and custom headers - * @returns Response status, headers, and parsed body + * @returns Response status, status text, headers, and parsed body * @throws {AuthError} Only on authentication failure (not on API errors) */ export async function rawApiRequest( endpoint: string, options: ApiRequestOptions & { headers?: Record } = {} -): Promise<{ status: number; headers: Headers; body: unknown }> { +): Promise<{ + status: number; + statusText: string; + headers: Headers; + body: unknown; +}> { const { method = "GET", body, params, headers: customHeaders = {} } = options; const config = getDefaultSdkConfig(); @@ -781,6 +786,7 @@ export async function rawApiRequest( return { status: response.status, + statusText: response.statusText, headers: response.headers, body: responseBody, }; diff --git a/packages/cli/test/commands/api.test.ts b/packages/cli/test/commands/api.test.ts index 0a29f4ffda..a4c9d570e7 100644 --- a/packages/cli/test/commands/api.test.ts +++ b/packages/cli/test/commands/api.test.ts @@ -19,6 +19,7 @@ import { extractJsonBody, formatApiResponse, formatBinaryErrorBody, + isApiErrorStatus, normalizeEndpoint, normalizeFields, parseDataBody, @@ -1028,6 +1029,113 @@ describe("resolveApiResponseOutput", () => { } }); + test("empty error body throws OutputError with the HTTP status and request", () => { + try { + resolveApiResponseOutput( + { + status: 404, + statusText: "Not Found", + headers: new Headers(), + body: "", + }, + { + silent: false, + isTTY: false, + method: "GET", + endpoint: "issues/7670740039/committers/", + } + ); + throw new Error("expected throw"); + } catch (error) { + expect(error).toBeInstanceOf(OutputError); + expect((error as OutputError).data).toBe( + "HTTP 404 Not Found — GET /api/0/issues/7670740039/committers/" + ); + } + }); + + test("whitespace-only error body throws OutputError with the HTTP status", () => { + try { + resolveApiResponseOutput( + { + status: 404, + statusText: "Not Found", + headers: new Headers(), + body: " \n", + }, + { + silent: false, + isTTY: false, + method: "GET", + endpoint: "missing/", + } + ); + throw new Error("expected throw"); + } catch (error) { + expect(error).toBeInstanceOf(OutputError); + expect((error as OutputError).data).toBe( + "HTTP 404 Not Found — GET /api/0/missing/" + ); + } + }); + + test.each([ + [199, "Informational"], + [300, "Multiple Choices"], + [304, "Not Modified"], + ])("non-2xx status %i throws OutputError", (status, statusText) => { + try { + resolveApiResponseOutput( + { + status, + statusText, + headers: new Headers(), + body: "", + }, + { + silent: false, + isTTY: false, + method: "GET", + endpoint: "missing/", + } + ); + throw new Error("expected throw"); + } catch (error) { + expect(error).toBeInstanceOf(OutputError); + expect((error as OutputError).data).toBe( + `HTTP ${status} ${statusText} — GET /api/0/missing/` + ); + } + }); + + test("JSON errors preserve the empty body in an HTTP response envelope", () => { + try { + resolveApiResponseOutput( + { + status: 404, + statusText: "Not Found", + headers: new Headers(), + body: "", + }, + { + silent: false, + isTTY: false, + json: true, + method: "GET", + endpoint: "missing/", + } + ); + throw new Error("expected throw"); + } catch (error) { + expect(error).toBeInstanceOf(OutputError); + expect((error as OutputError).data).toEqual({ + status: 404, + statusText: "Not Found", + body: "", + }); + } + }); + test("error + binary body throws OutputError with a byte summary, never raw bytes", () => { try { resolveApiResponseOutput( @@ -1074,6 +1182,17 @@ describe("resolveApiResponseOutput", () => { }); }); +describe("isApiErrorStatus", () => { + test.each([ + [199, true], + [200, false], + [299, false], + [300, true], + ])("classifies HTTP %i consistently", (status, expected) => { + expect(isApiErrorStatus(status)).toBe(expected); + }); +}); + describe("resolveBinaryTtyOutput", () => { let stderrOutput: string; let originalWrite: typeof process.stderr.write; diff --git a/packages/cli/test/e2e/api.test.ts b/packages/cli/test/e2e/api.test.ts index 4921b5b394..6433fee784 100644 --- a/packages/cli/test/e2e/api.test.ts +++ b/packages/cli/test/e2e/api.test.ts @@ -64,6 +64,63 @@ describe("sentry api", () => { expect(Array.isArray(data)).toBe(true); }); + test("--json includes the HTTP response envelope", async () => { + await ctx.setAuthToken(TEST_TOKEN); + + const result = await ctx.run(["api", "organizations/", "--json"]); + + expect(result.exitCode).toBe(0); + const data = JSON.parse(result.stdout); + expect(data).toMatchObject({ + status: 200, + statusText: "OK", + body: expect.any(Array), + }); + }); + + test("--json applies --fields to the API body inside the envelope", async () => { + await ctx.setAuthToken(TEST_TOKEN); + + const result = await ctx.run([ + "api", + "organizations/", + "--json", + "--fields", + "name", + ]); + + expect(result.exitCode, result.stderr + result.stdout).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + status: 200, + statusText: "OK", + body: [{ name: "Test Organization" }, { name: "Test Organization 2" }], + }); + }); + + test("empty error bodies report the HTTP status and request", async () => { + await ctx.setAuthToken(TEST_TOKEN); + + const result = await ctx.run(["api", "empty-error/"]); + + expect(result.exitCode).toBe(EXIT.OUTPUT_ERROR); + expect(result.stdout).toContain( + "HTTP 404 Not Found — GET /api/0/empty-error/" + ); + }); + + test("--json preserves empty error bodies in the response envelope", async () => { + await ctx.setAuthToken(TEST_TOKEN); + + const result = await ctx.run(["api", "empty-error/", "--json"]); + + expect(result.exitCode).toBe(EXIT.OUTPUT_ERROR); + expect(JSON.parse(result.stdout)).toEqual({ + status: 404, + statusText: "Not Found", + body: "", + }); + }); + test( "invalid endpoint returns non-zero exit code", { timeout: 15_000 }, @@ -179,7 +236,7 @@ describe("sentry api", () => { // Verbose output goes to stderr via logger.debug() // consola formats as: [debug] [api] > GET /api/0/organizations/ expect(result.stderr).toMatch(/> GET \/api\/0\/organizations\//); - expect(result.stderr).toMatch(/< HTTP \d{3}/); + expect(result.stderr).toMatch(/< HTTP 200 OK/); expect(result.stderr).toMatch(/< content-type:/i); // stdout should still contain the response body const data = JSON.parse(result.stdout); diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 0211b0b556..c9fdd3e1fd 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -669,4 +669,19 @@ describe("rawApiRequest binary handling", () => { const result = await rawApiRequest("some/text/"); expect(result.body).toBe("not json"); }); + + test("returns the HTTP status text with the response", async () => { + globalThis.fetch = mockFetch( + async () => + new Response("", { + status: 404, + statusText: "Not Found", + }) + ); + + const result = await rawApiRequest("missing/"); + expect(result.status).toBe(404); + expect(result.statusText).toBe("Not Found"); + expect(result.body).toBe(""); + }); }); diff --git a/packages/cli/test/mocks/routes.ts b/packages/cli/test/mocks/routes.ts index bf4cc56f8f..db06122df1 100644 --- a/packages/cli/test/mocks/routes.ts +++ b/packages/cli/test/mocks/routes.ts @@ -182,6 +182,11 @@ export const apiRoutes: MockRoute[] = [ path: "/api/0/organizations/", response: organizationsFixture, }, + { + method: "GET", + path: "/api/0/empty-error/", + response: () => ({ status: 404 }), + }, { method: "GET", path: "/api/0/organizations/:orgSlug/", From 3595d93c1228ec86ae52866ab12a6952695727c3 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 22 Aug 2026 12:25:16 +0000 Subject: [PATCH 2/2] fix(api): preserve dry-run field filtering --- packages/cli/src/commands/api.ts | 2 +- packages/cli/test/e2e/api.test.ts | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/api.ts b/packages/cli/src/commands/api.ts index 91912319b8..892d5f2b51 100644 --- a/packages/cli/src/commands/api.ts +++ b/packages/cli/src/commands/api.ts @@ -1023,7 +1023,7 @@ function formatApiResponseJson(data: unknown, fields?: string[]): unknown { !("status" in data) || !("body" in data) ) { - return data; + return fields && fields.length > 0 ? filterFields(data, fields) : data; } const response = data as ApiResponseOutput; diff --git a/packages/cli/test/e2e/api.test.ts b/packages/cli/test/e2e/api.test.ts index 6433fee784..d9705da80e 100644 --- a/packages/cli/test/e2e/api.test.ts +++ b/packages/cli/test/e2e/api.test.ts @@ -97,6 +97,22 @@ describe("sentry api", () => { }); }); + test("--dry-run --json applies --fields to the request preview", async () => { + await ctx.setAuthToken(TEST_TOKEN); + + const result = await ctx.run([ + "api", + "organizations/", + "--dry-run", + "--json", + "--fields", + "method", + ]); + + expect(result.exitCode, result.stderr + result.stdout).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ method: "GET" }); + }); + test("empty error bodies report the HTTP status and request", async () => { await ctx.setAuthToken(TEST_TOKEN);