Skip to content

Commit 31dfdf0

Browse files
committed
refactor: api command fully return-based, remove writeResponseBody
Both dry-run and normal response paths now return { data } through the output system. No more imperative writeJson/writeResponseBody calls. Changes: - Add exitCode field to CommandOutput<T> — the buildCommand wrapper calls process.exit() after rendering when set. This works around Stricli overwriting process.exitCode after the command returns. - Add formatApiResponse human formatter — preserves raw strings (plain text, HTML error pages) without JSON quoting, JSON-formats objects. - Api command uses output: { json: true, human: formatApiResponse } instead of flag-only output: 'json'. - Error responses return { data: body, exitCode: 1 } instead of calling process.exit(1) directly after imperative writes. - Remove writeResponseBody function and writeJson import from api.ts.
1 parent b307377 commit 31dfdf0

5 files changed

Lines changed: 203 additions & 84 deletions

File tree

src/commands/api.ts

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
import type { SentryContext } from "../context.js";
99
import { buildSearchParams, rawApiRequest } from "../lib/api-client.js";
1010
import { buildCommand } from "../lib/command.js";
11-
import { ValidationError } from "../lib/errors.js";
12-
import { writeJson } from "../lib/formatters/json.js";
11+
import { OutputError, ValidationError } from "../lib/errors.js";
1312
import { validateEndpoint } from "../lib/input-validation.js";
1413
import { getDefaultSdkConfig } from "../lib/sentry-client.js";
1514
import type { Writer } from "../types/index.js";
@@ -27,9 +26,9 @@ type ApiFlags = {
2726
readonly silent: boolean;
2827
readonly verbose: boolean;
2928
readonly "dry-run": boolean;
30-
/** Injected by buildCommand via output: "json" */
29+
/** Injected by buildCommand via output config */
3130
readonly json: boolean;
32-
/** Injected by buildCommand via output: "json" */
31+
/** Injected by buildCommand via output config */
3332
readonly fields?: string[];
3433
};
3534

@@ -865,28 +864,23 @@ export function writeResponseHeaders(
865864
}
866865

867866
/**
868-
* Write API response body to stdout.
867+
* Format an API response body for human-readable output.
869868
*
870-
* Preserves raw strings (plain text, HTML error pages) without JSON quoting.
871-
* Objects/arrays are JSON-formatted with optional `--fields` filtering.
872-
* Null/undefined bodies produce no output.
869+
* The api command is a raw proxy — the response body is the output.
870+
* Objects/arrays are JSON-formatted; strings (plain text, HTML error
871+
* pages) pass through without JSON quoting; null/undefined produce
872+
* no output.
873873
*
874874
* @internal Exported for testing
875875
*/
876-
export function writeResponseBody(
877-
stdout: Writer,
878-
body: unknown,
879-
fields?: string[]
880-
): void {
876+
export function formatApiResponse(body: unknown): string {
881877
if (body === null || body === undefined) {
882-
return;
878+
return "";
883879
}
884-
885880
if (typeof body === "object") {
886-
writeJson(stdout, body, fields);
887-
} else {
888-
stdout.write(`${String(body)}\n`);
881+
return JSON.stringify(body, null, 2);
889882
}
883+
return String(body);
890884
}
891885

892886
/**
@@ -1094,7 +1088,7 @@ export async function resolveBody(
10941088
// Command Definition
10951089

10961090
export const apiCommand = buildCommand({
1097-
output: "json",
1091+
output: { json: true, human: formatApiResponse },
10981092
docs: {
10991093
brief: "Make an authenticated API request",
11001094
fullDescription:
@@ -1219,17 +1213,14 @@ export const apiCommand = buildCommand({
12191213

12201214
// Dry-run mode: preview the request that would be sent
12211215
if (flags["dry-run"]) {
1222-
writeJson(
1223-
stdout,
1224-
{
1216+
return {
1217+
data: {
12251218
method: flags.method,
12261219
url: resolveRequestUrl(normalizedEndpoint, params),
12271220
headers: resolveEffectiveHeaders(headers, body),
12281221
body: body ?? null,
12291222
},
1230-
flags.fields
1231-
);
1232-
return;
1223+
};
12331224
}
12341225

12351226
// Verbose mode: show request details before the response
@@ -1254,22 +1245,19 @@ export const apiCommand = buildCommand({
12541245
return;
12551246
}
12561247

1257-
// Output response headers when requested
1248+
// Output response headers when requested (side-effect before body)
12581249
if (flags.verbose) {
12591250
writeVerboseResponse(stdout, response.status, response.headers);
12601251
} else if (flags.include) {
12611252
writeResponseHeaders(stdout, response.status, response.headers);
12621253
}
12631254

1264-
// Write response body — preserve raw strings, JSON-format objects.
1265-
// The api command is a raw proxy; non-JSON responses (plain text,
1266-
// HTML error pages) must not be wrapped in JSON string quotes.
1267-
writeResponseBody(stdout, response.body, flags.fields);
1268-
1269-
// Error exit: Stricli overwrites process.exitCode after the command
1270-
// returns, so we must call process.exit(1) directly.
1255+
// Error responses: throw so the wrapper renders the body then exits 1.
1256+
// The body is still useful (API error details), just indicates failure.
12711257
if (isError) {
1272-
process.exit(1);
1258+
throw new OutputError(response.body);
12731259
}
1260+
1261+
return { data: response.body };
12741262
},
12751263
});

src/lib/command.ts

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
buildCommand as stricliCommand,
3737
numberParser as stricliNumberParser,
3838
} from "@stricli/core";
39+
import { OutputError } from "./errors.js";
3940
import { parseFieldsList } from "./formatters/json.js";
4041
import {
4142
type CommandOutput,
@@ -379,19 +380,43 @@ export function buildCommand<
379380
setArgsContext(args);
380381
}
381382

383+
// OutputError handler: render data through the output system, then
384+
// exit with the error's code. Stricli overwrites process.exitCode = 0
385+
// after successful returns, so process.exit() is the only way to
386+
// preserve a non-zero code. This lives in the framework — commands
387+
// simply `throw new OutputError(data)`.
388+
const handleOutputError = (err: unknown): never => {
389+
if (err instanceof OutputError && outputConfig) {
390+
handleReturnValue(
391+
this,
392+
{ data: err.data } as CommandOutput<unknown>,
393+
cleanFlags
394+
);
395+
process.exit(err.exitCode);
396+
}
397+
throw err;
398+
};
399+
382400
// Call original and intercept data returns.
383401
// Commands with output config return { data, hint? };
384402
// the wrapper renders automatically. Void returns are ignored.
385-
const result = originalFunc.call(
386-
this,
387-
cleanFlags as FLAGS,
388-
...(args as unknown as ARGS)
389-
);
403+
let result: ReturnType<typeof originalFunc>;
404+
try {
405+
result = originalFunc.call(
406+
this,
407+
cleanFlags as FLAGS,
408+
...(args as unknown as ARGS)
409+
);
410+
} catch (err) {
411+
handleOutputError(err);
412+
}
390413

391414
if (result instanceof Promise) {
392-
return result.then((resolved) => {
393-
handleReturnValue(this, resolved, cleanFlags);
394-
}) as ReturnType<typeof originalFunc>;
415+
return result
416+
.then((resolved) => {
417+
handleReturnValue(this, resolved, cleanFlags);
418+
})
419+
.catch(handleOutputError) as ReturnType<typeof originalFunc>;
395420
}
396421

397422
handleReturnValue(this, result, cleanFlags);

src/lib/errors.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,26 @@ export class ConfigError extends CliError {
130130
}
131131
}
132132

133+
/**
134+
* Thrown when a command produces valid output but should exit non-zero.
135+
*
136+
* Unlike other errors, the output data is rendered to stdout (not stderr)
137+
* through the normal output system — the `buildCommand` wrapper catches
138+
* this before it reaches the global error handler. Think "HTTP 404 body":
139+
* useful data, but the operation itself failed.
140+
*
141+
* @param data - The output data to render (same type as CommandOutput.data)
142+
*/
143+
export class OutputError extends CliError {
144+
readonly data: unknown;
145+
146+
constructor(data: unknown) {
147+
super("", 1);
148+
this.name = "OutputError";
149+
this.data = data;
150+
}
151+
}
152+
133153
const DEFAULT_CONTEXT_ALTERNATIVES = [
134154
"Run from a directory with a Sentry-configured project",
135155
"Set SENTRY_ORG and SENTRY_PROJECT (or SENTRY_DSN) environment variables",

test/commands/api.test.ts

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
buildRawQueryParams,
1717
dataToQueryParams,
1818
extractJsonBody,
19+
formatApiResponse,
1920
normalizeEndpoint,
2021
normalizeFields,
2122
parseDataBody,
@@ -28,7 +29,6 @@ import {
2829
resolveEffectiveHeaders,
2930
resolveRequestUrl,
3031
setNestedValue,
31-
writeResponseBody,
3232
writeResponseHeaders,
3333
writeVerboseRequest,
3434
writeVerboseResponse,
@@ -913,51 +913,33 @@ describe("writeResponseHeaders", () => {
913913
});
914914
});
915915

916-
describe("writeResponseBody", () => {
917-
test("writes JSON object with pretty-printing", () => {
918-
const writer = createMockWriter();
919-
writeResponseBody(writer, { key: "value", num: 42 });
920-
expect(writer.output).toBe('{\n "key": "value",\n "num": 42\n}\n');
921-
});
922-
923-
test("writes JSON array with pretty-printing", () => {
924-
const writer = createMockWriter();
925-
writeResponseBody(writer, [1, 2, 3]);
926-
expect(writer.output).toBe("[\n 1,\n 2,\n 3\n]\n");
916+
describe("formatApiResponse", () => {
917+
test("formats JSON object with pretty-printing", () => {
918+
expect(formatApiResponse({ key: "value", num: 42 })).toBe(
919+
'{\n "key": "value",\n "num": 42\n}'
920+
);
927921
});
928922

929-
test("writes string directly without JSON quoting", () => {
930-
const writer = createMockWriter();
931-
writeResponseBody(writer, "plain text response");
932-
expect(writer.output).toBe("plain text response\n");
923+
test("formats JSON array with pretty-printing", () => {
924+
expect(formatApiResponse([1, 2, 3])).toBe("[\n 1,\n 2,\n 3\n]");
933925
});
934926

935-
test("writes number as string", () => {
936-
const writer = createMockWriter();
937-
writeResponseBody(writer, 42);
938-
expect(writer.output).toBe("42\n");
927+
test("formats string directly without JSON quoting", () => {
928+
expect(formatApiResponse("plain text response")).toBe(
929+
"plain text response"
930+
);
939931
});
940932

941-
test("does not write null", () => {
942-
const writer = createMockWriter();
943-
writeResponseBody(writer, null);
944-
expect(writer.output).toBe("");
933+
test("formats number as string", () => {
934+
expect(formatApiResponse(42)).toBe("42");
945935
});
946936

947-
test("does not write undefined", () => {
948-
const writer = createMockWriter();
949-
writeResponseBody(writer, undefined);
950-
expect(writer.output).toBe("");
937+
test("returns empty string for null", () => {
938+
expect(formatApiResponse(null)).toBe("");
951939
});
952940

953-
test("applies --fields filtering to objects", () => {
954-
const writer = createMockWriter();
955-
writeResponseBody(writer, { id: "1", name: "test", extra: "data" }, [
956-
"id",
957-
"name",
958-
]);
959-
const parsed = JSON.parse(writer.output);
960-
expect(parsed).toEqual({ id: "1", name: "test" });
941+
test("returns empty string for undefined", () => {
942+
expect(formatApiResponse(undefined)).toBe("");
961943
});
962944
});
963945

0 commit comments

Comments
 (0)