Skip to content

Commit c06fe6f

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 c06fe6f

File tree

5 files changed

+135
-71
lines changed

5 files changed

+135
-71
lines changed

src/commands/api.ts

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import type { SentryContext } from "../context.js";
99
import { buildSearchParams, rawApiRequest } from "../lib/api-client.js";
1010
import { buildCommand } from "../lib/command.js";
1111
import { ValidationError } from "../lib/errors.js";
12-
import { writeJson } from "../lib/formatters/json.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,16 @@ 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.
1271-
if (isError) {
1272-
process.exit(1);
1273-
}
1255+
return {
1256+
data: response.body,
1257+
exitCode: isError ? 1 : undefined,
1258+
};
12741259
},
12751260
});

src/lib/command.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,12 @@ export function buildCommand<
338338
json: Boolean(flags.json),
339339
fields: flags.fields as string[] | undefined,
340340
});
341+
342+
// Honor exit code AFTER rendering — Stricli overwrites process.exitCode
343+
// after the func returns, so we must use process.exit() directly.
344+
if (typeof value.exitCode === "number") {
345+
process.exit(value.exitCode);
346+
}
341347
}
342348

343349
/**

src/lib/formatters/output.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,15 @@ export type CommandOutput<T> = {
102102
data: T;
103103
/** Hint line appended after human output (suppressed in JSON mode) */
104104
hint?: string;
105+
/**
106+
* Exit code to set after rendering output.
107+
*
108+
* Stricli overwrites `process.exitCode` after the command returns,
109+
* so commands that need non-zero exit (e.g. API error responses)
110+
* must signal it here. The wrapper calls `process.exit()` after
111+
* rendering is complete.
112+
*/
113+
exitCode?: number;
105114
};
106115

107116
/**

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

test/lib/command.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,4 +1236,86 @@ describe("buildCommand return-based output", () => {
12361236
expect(jsonRaw).not.toContain("Detected from");
12371237
expect(JSON.parse(jsonRaw)).toEqual({ org: "sentry" });
12381238
});
1239+
1240+
test("exitCode calls process.exit after rendering", async () => {
1241+
let exitCalledWith: number | undefined;
1242+
const originalExit = process.exit;
1243+
1244+
const command = buildCommand<
1245+
{ json: boolean; fields?: string[] },
1246+
[],
1247+
TestContext
1248+
>({
1249+
docs: { brief: "Test" },
1250+
output: {
1251+
json: true,
1252+
human: (d: { error: string }) => `Error: ${d.error}`,
1253+
},
1254+
parameters: {},
1255+
func(this: TestContext) {
1256+
return { data: { error: "not found" }, exitCode: 1 };
1257+
},
1258+
});
1259+
1260+
const routeMap = buildRouteMap({
1261+
routes: { test: command },
1262+
docs: { brief: "Test app" },
1263+
});
1264+
const app = buildApplication(routeMap, { name: "test" });
1265+
const ctx = createTestContext();
1266+
1267+
// Mock process.exit to capture the call without actually exiting
1268+
process.exit = ((code?: number) => {
1269+
exitCalledWith = code;
1270+
}) as typeof process.exit;
1271+
1272+
try {
1273+
await run(app, ["test"], ctx as TestContext);
1274+
expect(exitCalledWith).toBe(1);
1275+
// Output was rendered BEFORE exit
1276+
expect(ctx.output.join("")).toContain("Error: not found");
1277+
} finally {
1278+
process.exit = originalExit;
1279+
}
1280+
});
1281+
1282+
test("exitCode is not called when undefined", async () => {
1283+
let exitCalled = false;
1284+
const originalExit = process.exit;
1285+
1286+
const command = buildCommand<
1287+
{ json: boolean; fields?: string[] },
1288+
[],
1289+
TestContext
1290+
>({
1291+
docs: { brief: "Test" },
1292+
output: {
1293+
json: true,
1294+
human: (d: { ok: boolean }) => `OK: ${d.ok}`,
1295+
},
1296+
parameters: {},
1297+
func(this: TestContext) {
1298+
return { data: { ok: true } };
1299+
},
1300+
});
1301+
1302+
const routeMap = buildRouteMap({
1303+
routes: { test: command },
1304+
docs: { brief: "Test app" },
1305+
});
1306+
const app = buildApplication(routeMap, { name: "test" });
1307+
const ctx = createTestContext();
1308+
1309+
process.exit = (() => {
1310+
exitCalled = true;
1311+
}) as typeof process.exit;
1312+
1313+
try {
1314+
await run(app, ["test"], ctx as TestContext);
1315+
expect(exitCalled).toBe(false);
1316+
expect(ctx.output.join("")).toContain("OK: true");
1317+
} finally {
1318+
process.exit = originalExit;
1319+
}
1320+
});
12391321
});

0 commit comments

Comments
 (0)