Skip to content

Commit 6980508

Browse files
committed
refactor: api command uses return-based output, remove buildDryRunRequest
Both code paths now return { data } through the standard output system: - Dry-run: returns { data: { method, url, headers, body } } preview - Normal: returns { data: response.body } after side-effect writes (verbose/include headers, error exit code) Key changes: - output: 'json' → { json: true } (JSON-only config, no human formatter) - Make OutputConfig.human optional — when absent, renderCommandOutput always serializes as JSON regardless of --json flag - Remove buildDryRunRequest — inline request preview construction using resolveRequestUrl + new resolveEffectiveHeaders helper - Remove handleResponse — its behaviors are inlined in func: verbose/include headers as pre-return side effects, silent mode returns void, error uses process.exitCode (not process.exit) so the output wrapper can render before the process exits - Remove DryRunRequest type, writeJson import Tests: replace handleResponse tests (8) and buildDryRunRequest tests (4+5 property) with resolveEffectiveHeaders tests (6+4 property). 288 tests pass across 3 files.
1 parent 70e7f2d commit 6980508

File tree

4 files changed

+133
-336
lines changed

4 files changed

+133
-336
lines changed

src/commands/api.ts

Lines changed: 48 additions & 94 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: { json: true } */
3130
readonly json: boolean;
32-
/** Injected by buildCommand via output: "json" */
31+
/** Injected by buildCommand via output: { json: true } */
3332
readonly fields?: string[];
3433
};
3534

@@ -938,95 +937,27 @@ export function resolveRequestUrl(
938937
}
939938

940939
/**
941-
* Dry-run request details — everything that would be sent without actually sending.
942-
*/
943-
export type DryRunRequest = {
944-
method: string;
945-
url: string;
946-
headers: Record<string, string>;
947-
body: unknown;
948-
};
949-
950-
/**
951-
* Build a DryRunRequest from the resolved request components.
940+
* Resolve effective request headers, mirroring rawApiRequest logic.
952941
*
953-
* Mirrors the URL and header logic from rawApiRequest so the
954-
* preview matches what the real request would look like.
942+
* Auto-adds Content-Type: application/json for non-string object bodies
943+
* when no Content-Type was explicitly provided.
955944
*
956945
* @internal Exported for testing
957946
*/
958-
export function buildDryRunRequest(
959-
method: string,
960-
endpoint: string,
961-
options: {
962-
params?: Record<string, string | string[]>;
963-
headers?: Record<string, string>;
964-
body?: unknown;
965-
}
966-
): DryRunRequest {
967-
const headers = { ...(options.headers ?? {}) };
968-
969-
// Mirror rawApiRequest: auto-add Content-Type for object bodies
970-
// when no Content-Type was explicitly provided
947+
export function resolveEffectiveHeaders(
948+
customHeaders: Record<string, string> | undefined,
949+
body: unknown
950+
): Record<string, string> {
951+
const headers = { ...(customHeaders ?? {}) };
971952
if (
972-
options.body !== undefined &&
973-
options.body !== null &&
974-
typeof options.body !== "string" &&
953+
body !== undefined &&
954+
body !== null &&
955+
typeof body !== "string" &&
975956
!Object.keys(headers).some((k) => k.toLowerCase() === "content-type")
976957
) {
977958
headers["Content-Type"] = "application/json";
978959
}
979-
980-
return {
981-
method,
982-
url: resolveRequestUrl(endpoint, options.params),
983-
headers,
984-
body: options.body ?? null,
985-
};
986-
}
987-
988-
/**
989-
* Handle response output based on flags
990-
* @internal Exported for testing
991-
*/
992-
export function handleResponse(
993-
stdout: Writer,
994-
response: { status: number; headers: Headers; body: unknown },
995-
flags: {
996-
silent: boolean;
997-
verbose: boolean;
998-
include: boolean;
999-
fields?: string[];
1000-
}
1001-
): void {
1002-
const isError = response.status >= 400;
1003-
1004-
// Silent mode - only set exit code
1005-
if (flags.silent) {
1006-
if (isError) {
1007-
process.exit(1);
1008-
}
1009-
return;
1010-
}
1011-
1012-
// Output headers (verbose or include mode)
1013-
if (flags.verbose) {
1014-
writeVerboseResponse(stdout, response.status, response.headers);
1015-
} else if (flags.include) {
1016-
writeResponseHeaders(stdout, response.status, response.headers);
1017-
}
1018-
1019-
// Output body — apply --fields filtering when requested
1020-
if (flags.fields && flags.fields.length > 0) {
1021-
writeJson(stdout, response.body, flags.fields);
1022-
} else {
1023-
writeResponseBody(stdout, response.body);
1024-
}
1025-
1026-
// Exit with error code for error responses
1027-
if (isError) {
1028-
process.exit(1);
1029-
}
960+
return headers;
1030961
}
1031962

1032963
/**
@@ -1152,7 +1083,7 @@ export async function resolveBody(
11521083
// Command Definition
11531084

11541085
export const apiCommand = buildCommand({
1155-
output: "json",
1086+
output: { json: true },
11561087
docs: {
11571088
brief: "Make an authenticated API request",
11581089
fullDescription:
@@ -1275,18 +1206,19 @@ export const apiCommand = buildCommand({
12751206
? parseHeaders(flags.header)
12761207
: undefined;
12771208

1278-
// Dry-run mode: show the resolved request without sending it
1209+
// Dry-run mode: preview the request that would be sent
12791210
if (flags["dry-run"]) {
1280-
const request = buildDryRunRequest(flags.method, normalizedEndpoint, {
1281-
params,
1282-
headers,
1283-
body,
1284-
});
1285-
writeJson(stdout, request, flags.fields);
1286-
return;
1211+
return {
1212+
data: {
1213+
method: flags.method,
1214+
url: resolveRequestUrl(normalizedEndpoint, params),
1215+
headers: resolveEffectiveHeaders(headers, body),
1216+
body: body ?? null,
1217+
},
1218+
};
12871219
}
12881220

1289-
// Verbose mode: show request details (unless silent)
1221+
// Verbose mode: show request details before the response
12901222
if (flags.verbose && !flags.silent) {
12911223
writeVerboseRequest(stdout, flags.method, normalizedEndpoint, headers);
12921224
}
@@ -1298,6 +1230,28 @@ export const apiCommand = buildCommand({
12981230
headers,
12991231
});
13001232

1301-
handleResponse(stdout, response, flags);
1233+
const isError = response.status >= 400;
1234+
1235+
// Silent mode — only set exit code, no output
1236+
if (flags.silent) {
1237+
if (isError) {
1238+
process.exit(1);
1239+
}
1240+
return;
1241+
}
1242+
1243+
// Output response headers when requested
1244+
if (flags.verbose) {
1245+
writeVerboseResponse(stdout, response.status, response.headers);
1246+
} else if (flags.include) {
1247+
writeResponseHeaders(stdout, response.status, response.headers);
1248+
}
1249+
1250+
// Set error exit code (before return so the process exits after output)
1251+
if (isError) {
1252+
process.exitCode = 1;
1253+
}
1254+
1255+
return { data: response.body };
13021256
},
13031257
});

src/lib/formatters/output.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type WriteOutputOptions<T> = {
6060
/**
6161
* Output configuration declared on `buildCommand` for automatic rendering.
6262
*
63-
* Two forms:
63+
* Three forms:
6464
*
6565
* 1. **Flag-only** — `output: "json"` — injects `--json` and `--fields` flags
6666
* but does not intercept returns. Commands handle their own output.
@@ -69,14 +69,23 @@ type WriteOutputOptions<T> = {
6969
* AND auto-renders the command's return value. Commands return
7070
* `{ data }` or `{ data, hint }` objects.
7171
*
72+
* 3. **JSON-only config** — `output: { json: true }` — like full config but
73+
* without a `human` formatter. Data is always serialized as JSON.
74+
*
7275
* @typeParam T - Type of data the command returns (used by `human` formatter
7376
* and serialized as-is to JSON)
7477
*/
7578
export type OutputConfig<T> = {
7679
/** Enable `--json` and `--fields` flag injection */
7780
json: true;
78-
/** Format data as a human-readable string for terminal output */
79-
human: (data: T) => string;
81+
/**
82+
* Format data as a human-readable string for terminal output.
83+
*
84+
* When omitted the command is **JSON-only**: data is always serialized
85+
* as JSON regardless of whether `--json` was passed. The `--json` and
86+
* `--fields` flags are still injected for consistency.
87+
*/
88+
human?: (data: T) => string;
8089
/**
8190
* Top-level keys to strip from JSON output.
8291
*
@@ -136,7 +145,8 @@ export function renderCommandOutput(
136145
config: OutputConfig<any>,
137146
ctx: RenderContext
138147
): void {
139-
if (ctx.json) {
148+
// JSON mode: explicit --json flag, or no human formatter (JSON-only command)
149+
if (ctx.json || !config.human) {
140150
let jsonData = data;
141151
if (
142152
config.jsonExclude &&

test/commands/api.property.test.ts

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ import {
1616
oneof,
1717
property,
1818
record,
19+
string,
1920
stringMatching,
2021
tuple,
2122
uniqueArray,
2223
} from "fast-check";
2324
import {
24-
buildDryRunRequest,
2525
buildFromFields,
2626
extractJsonBody,
2727
normalizeEndpoint,
@@ -31,6 +31,7 @@ import {
3131
parseFieldValue,
3232
parseMethod,
3333
resolveBody,
34+
resolveEffectiveHeaders,
3435
resolveRequestUrl,
3536
setNestedValue,
3637
} from "../../src/commands/api.js";
@@ -794,7 +795,6 @@ describe("property: resolveBody", () => {
794795
// Dry-run property tests
795796

796797
/** Arbitrary for HTTP methods */
797-
const httpMethodArb = constantFrom("GET", "POST", "PUT", "DELETE", "PATCH");
798798

799799
/** Arbitrary for clean API endpoint paths (no query string, for dry-run tests) */
800800
const dryRunEndpointArb = stringMatching(
@@ -855,61 +855,65 @@ describe("property: resolveRequestUrl", () => {
855855
});
856856
});
857857

858-
describe("property: buildDryRunRequest", () => {
859-
test("method is preserved exactly", () => {
860-
fcAssert(
861-
property(httpMethodArb, dryRunEndpointArb, (method, endpoint) => {
862-
const request = buildDryRunRequest(method, endpoint, {});
863-
expect(request.method).toBe(method);
864-
}),
865-
{ numRuns: DEFAULT_NUM_RUNS }
866-
);
858+
describe("property: resolveEffectiveHeaders", () => {
859+
/** Arbitrary for custom header entries (non-content-type keys) */
860+
const headerKeyArb = stringMatching(/^X-[A-Za-z]{1,10}$/);
861+
const headerValueArb = string({ minLength: 1, maxLength: 20 });
862+
const customHeadersArb = dictionary(headerKeyArb, headerValueArb, {
863+
minKeys: 0,
864+
maxKeys: 3,
867865
});
868866

869-
test("URL contains the endpoint", () => {
867+
test("preserves all custom headers", () => {
870868
fcAssert(
871-
property(httpMethodArb, dryRunEndpointArb, (method, endpoint) => {
872-
const request = buildDryRunRequest(method, endpoint, {});
873-
const normalized = endpoint.startsWith("/")
874-
? endpoint.slice(1)
875-
: endpoint;
876-
expect(request.url).toContain(normalized);
869+
property(customHeadersArb, (custom) => {
870+
const result = resolveEffectiveHeaders(custom, undefined);
871+
for (const [key, value] of Object.entries(custom)) {
872+
expect(result[key]).toBe(value);
873+
}
877874
}),
878875
{ numRuns: DEFAULT_NUM_RUNS }
879876
);
880877
});
881878

882-
test("headers default to empty object when undefined", () => {
879+
test("auto-adds Content-Type for object bodies without it", () => {
883880
fcAssert(
884-
property(httpMethodArb, dryRunEndpointArb, (method, endpoint) => {
885-
const request = buildDryRunRequest(method, endpoint, {});
886-
expect(request.headers).toEqual({});
881+
property(customHeadersArb, jsonValue(), (custom, body) => {
882+
// Ensure body is a non-null object (not string/number/boolean/null)
883+
if (body === null || body === undefined || typeof body !== "object") {
884+
return;
885+
}
886+
const result = resolveEffectiveHeaders(custom, body);
887+
expect(result["Content-Type"]).toBe("application/json");
887888
}),
888889
{ numRuns: DEFAULT_NUM_RUNS }
889890
);
890891
});
891892

892-
test("body defaults to null when undefined", () => {
893+
test("never adds Content-Type for string bodies", () => {
893894
fcAssert(
894-
property(httpMethodArb, dryRunEndpointArb, (method, endpoint) => {
895-
const request = buildDryRunRequest(method, endpoint, {});
896-
expect(request.body).toBeNull();
895+
property(customHeadersArb, string(), (custom, body) => {
896+
const result = resolveEffectiveHeaders(custom, body);
897+
// Should not have auto-added Content-Type (only custom headers)
898+
if (!custom["Content-Type"]) {
899+
expect(result["Content-Type"]).toBeUndefined();
900+
}
897901
}),
898902
{ numRuns: DEFAULT_NUM_RUNS }
899903
);
900904
});
901905

902-
test("provided body is preserved", () => {
906+
test("does not override explicit Content-Type", () => {
907+
const contentTypeArb = constantFrom(
908+
"text/plain",
909+
"text/xml",
910+
"application/x-www-form-urlencoded"
911+
);
903912
fcAssert(
904-
property(
905-
httpMethodArb,
906-
dryRunEndpointArb,
907-
jsonValue(),
908-
(method, endpoint, body) => {
909-
const request = buildDryRunRequest(method, endpoint, { body });
910-
expect(request.body).toEqual(body);
911-
}
912-
),
913+
property(contentTypeArb, jsonValue(), (ct, body) => {
914+
const result = resolveEffectiveHeaders({ "Content-Type": ct }, body);
915+
expect(result["Content-Type"]).toBe(ct);
916+
}),
913917
{ numRuns: DEFAULT_NUM_RUNS }
914918
);
915919
});

0 commit comments

Comments
 (0)