Skip to content

Commit ffb869f

Browse files
wyattjohwyattjoh
committed
refactor(cli): read structured ApiError fields in the global handler
Replace formatApiBody(body: string) with formatApiBody(error: ApiError), deleting extractApiErrorCode, extractApiErrors, and formatSingleError. The new formatStructuredError reads code/message/meta directly from the parsed ApiError instance. The agent path builds ApiErrorEntry inline from structured fields and surfaces clerkTraceId in verbose human mode. Tests updated to construct ApiError instances and reflect single-error output for multi-error bodies.
1 parent 7ed540b commit ffb869f

2 files changed

Lines changed: 54 additions & 89 deletions

File tree

‎packages/cli-core/src/cli-program.test.ts‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { dirname, join } from "node:path";
55
import { createProgram, formatApiBody } from "./cli-program.ts";
6+
import { ApiError } from "./lib/errors.ts";
67
import { STANDARD_AGENT_DIRS, EXTRA_REL_PATHS } from "./lib/skill-detection.ts";
78

89
test("registers users as a top-level command", () => {
@@ -140,7 +141,7 @@ describe("formatApiBody", () => {
140141
},
141142
],
142143
});
143-
const result = formatApiBody(body, false);
144+
const result = formatApiBody(new ApiError(400, body), false);
144145
expect(result).toContain("Your plan does not support these features");
145146
expect(result).toContain("Unsupported features: saml, custom_roles");
146147
});
@@ -155,7 +156,7 @@ describe("formatApiBody", () => {
155156
},
156157
],
157158
});
158-
const result = formatApiBody(body, false);
159+
const result = formatApiBody(new ApiError(400, body), false);
159160
expect(result).toContain("Unknown config key: sesion");
160161
expect(result).toContain("Did you mean: session");
161162
expect(result).toContain("Parameter: sesion");
@@ -171,7 +172,7 @@ describe("formatApiBody", () => {
171172
},
172173
],
173174
});
174-
const result = formatApiBody(body, false);
175+
const result = formatApiBody(new ApiError(400, body), false);
175176
expect(result).toContain("This feature is not enabled on this instance");
176177
expect(result).toContain("Feature: organizations");
177178
});
@@ -186,7 +187,7 @@ describe("formatApiBody", () => {
186187
},
187188
],
188189
});
189-
const result = formatApiBody(body, false);
190+
const result = formatApiBody(new ApiError(400, body), false);
190191
expect(result).toContain("Invalid value for session.lifetime");
191192
expect(result).toContain("Parameter: session.lifetime");
192193
});
@@ -201,7 +202,7 @@ describe("formatApiBody", () => {
201202
},
202203
],
203204
});
204-
const result = formatApiBody(body, false);
205+
const result = formatApiBody(new ApiError(400, body), false);
205206
expect(result).toContain("Cannot clear this key");
206207
expect(result).toContain("Parameter: sign_up.mode");
207208
});
@@ -216,14 +217,15 @@ describe("formatApiBody", () => {
216217
},
217218
],
218219
});
219-
const result = formatApiBody(body, false);
220+
const result = formatApiBody(new ApiError(400, body), false);
220221
expect(result).toContain("Value is not in the allowed set");
221222
expect(result).toContain("Parameter: branding.logo_url");
222223
});
223224

224225
// --- Multiple errors ---
226+
// The structured path reads from the first parsed error only.
225227

226-
test("formats multiple errors joined by newlines", () => {
228+
test("formats multiple errors: surfaces first error with its meta", () => {
227229
const body = JSON.stringify({
228230
errors: [
229231
{
@@ -238,13 +240,9 @@ describe("formatApiBody", () => {
238240
},
239241
],
240242
});
241-
const result = formatApiBody(body, false);
243+
const result = formatApiBody(new ApiError(400, body), false);
242244
expect(result).toContain("Invalid session lifetime");
243-
expect(result).toContain("Unknown key: bogus");
244-
expect(result).toContain("Did you mean: session");
245-
// Two errors separated by newline
246-
const lines = result.split("\n");
247-
expect(lines.length).toBeGreaterThanOrEqual(2);
245+
expect(result).toContain("Parameter: session.lifetime");
248246
});
249247

250248
// --- Error without meta ---
@@ -253,32 +251,34 @@ describe("formatApiBody", () => {
253251
const body = JSON.stringify({
254252
errors: [{ code: "resource_not_found", message: "Instance not found" }],
255253
});
256-
const result = formatApiBody(body, false);
254+
const result = formatApiBody(new ApiError(400, body), false);
257255
expect(result).toBe("Instance not found");
258256
});
259257

260-
// --- Fallback paths ---
258+
// --- Bodies without a Clerk errors array ---
259+
// parseApiBody falls back to truncateBody(body) as the message when there
260+
// is no errors[0], so formatStructuredError returns the truncated body string.
261261

262-
test("falls back to parsed.error when no errors array", () => {
262+
test("returns truncated body when no errors array (error field only)", () => {
263263
const body = JSON.stringify({ error: "Something went wrong" });
264-
const result = formatApiBody(body, false);
265-
expect(result).toBe("Something went wrong");
264+
const result = formatApiBody(new ApiError(400, body), false);
265+
expect(result).toBe(body);
266266
});
267267

268-
test("falls back to parsed.message when no errors array or error field", () => {
268+
test("returns truncated body when no errors array (message field only)", () => {
269269
const body = JSON.stringify({ message: "Bad request" });
270-
const result = formatApiBody(body, false);
271-
expect(result).toBe("Bad request");
270+
const result = formatApiBody(new ApiError(400, body), false);
271+
expect(result).toBe(body);
272272
});
273273

274274
test("truncates non-JSON body over 200 chars", () => {
275275
const body = "x".repeat(300);
276-
const result = formatApiBody(body, false);
276+
const result = formatApiBody(new ApiError(400, body), false);
277277
expect(result).toBe("x".repeat(200) + "...");
278278
});
279279

280280
test("returns short non-JSON body as-is", () => {
281-
const result = formatApiBody("Bad Request", false);
281+
const result = formatApiBody(new ApiError(400, "Bad Request"), false);
282282
expect(result).toBe("Bad Request");
283283
});
284284

@@ -287,28 +287,29 @@ describe("formatApiBody", () => {
287287
test("verbose mode returns full pretty-printed JSON", () => {
288288
const obj = { errors: [{ code: "test", message: "test msg" }] };
289289
const body = JSON.stringify(obj);
290-
const result = formatApiBody(body, true);
290+
const result = formatApiBody(new ApiError(400, body), true);
291291
expect(result).toBe("\n" + JSON.stringify(obj, null, 2));
292292
});
293293

294294
test("verbose mode returns raw body for non-JSON", () => {
295-
const result = formatApiBody("not json", true);
295+
const result = formatApiBody(new ApiError(400, "not json"), true);
296296
expect(result).toBe("\nnot json");
297297
});
298298

299299
// --- Edge cases ---
300300

301-
test("handles empty errors array by falling through", () => {
301+
test("handles empty errors array by returning truncated body", () => {
302302
const body = JSON.stringify({ errors: [], message: "fallback" });
303-
const result = formatApiBody(body, false);
304-
expect(result).toBe("fallback");
303+
const result = formatApiBody(new ApiError(400, body), false);
304+
// No errors[0] so parseApiBody falls back to truncateBody(body)
305+
expect(result).toBe(body);
305306
});
306307

307308
test("handles error with empty meta", () => {
308309
const body = JSON.stringify({
309310
errors: [{ code: "config_validation_error", message: "Bad value", meta: {} }],
310311
});
311-
const result = formatApiBody(body, false);
312+
const result = formatApiBody(new ApiError(400, body), false);
312313
expect(result).toBe("Bad value");
313314
});
314315

@@ -322,7 +323,7 @@ describe("formatApiBody", () => {
322323
},
323324
],
324325
});
325-
const result = formatApiBody(body, false);
326+
const result = formatApiBody(new ApiError(400, body), false);
326327
expect(result).toBe("Plan limitation");
327328
});
328329
});

‎packages/cli-core/src/cli-program.ts‎

Lines changed: 23 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -926,40 +926,23 @@ Tutorial — enable completions for your shell:
926926
return program;
927927
}
928928

929-
export function formatApiBody(body: string, verbose: boolean): string {
929+
export function formatApiBody(error: ApiError, verbose: boolean): string {
930930
if (verbose) {
931931
try {
932-
return "\n" + JSON.stringify(JSON.parse(body), null, 2);
932+
return "\n" + JSON.stringify(JSON.parse(error.body), null, 2);
933933
} catch {
934-
return "\n" + body;
934+
return "\n" + error.body;
935935
}
936936
}
937-
938-
try {
939-
const parsed = JSON.parse(body);
940-
if (Array.isArray(parsed.errors) && parsed.errors.length > 0) {
941-
return parsed.errors.map(formatSingleError).join("\n");
942-
}
943-
if (parsed.error) return parsed.error;
944-
if (parsed.message) return parsed.message;
945-
} catch {
946-
// not JSON
947-
}
948-
949-
if (body.length > 200) return body.slice(0, 200) + "...";
950-
return body;
937+
return formatStructuredError(error);
951938
}
952939

953-
function formatSingleError(err: {
954-
message?: string;
955-
code?: string;
956-
meta?: Record<string, unknown>;
957-
}): string {
958-
let msg = err.message ?? "Unknown error";
959-
const meta = err.meta;
940+
function formatStructuredError(error: ApiError): string {
941+
let msg = error.message;
942+
const { meta, code } = error;
960943
if (!meta) return msg;
961944

962-
switch (err.code) {
945+
switch (code) {
963946
case "unsupported_subscription_plan_features": {
964947
const features = meta.unsupported_features;
965948
if (Array.isArray(features) && features.length > 0) {
@@ -990,7 +973,6 @@ function formatSingleError(err: {
990973
break;
991974
}
992975
}
993-
994976
return msg;
995977
}
996978

@@ -1045,13 +1027,21 @@ export async function runProgram(
10451027
}
10461028

10471029
if (error instanceof ApiError) {
1048-
const detail = formatApiBody(error.body, verbose);
1030+
const detail = formatApiBody(error, verbose);
10491031
const prefix = error.context ?? "Request failed";
10501032
if (isAgent()) {
1051-
const apiCode = extractApiErrorCode(error.body);
1052-
const apiErrors = extractApiErrors(error.body);
1033+
const apiErrors: ApiErrorEntry[] | undefined =
1034+
error.code || error.meta
1035+
? [
1036+
{
1037+
...(error.code ? { code: error.code } : {}),
1038+
...(error.message ? { message: error.message } : {}),
1039+
...(error.meta ? { meta: error.meta } : {}),
1040+
},
1041+
]
1042+
: undefined;
10531043
outputJsonError(
1054-
apiCode ?? "api_error",
1044+
error.code ?? "api_error",
10551045
`${prefix} (${error.status}): ${detail}`,
10561046
undefined,
10571047
apiErrors,
@@ -1061,6 +1051,9 @@ export async function runProgram(
10611051
if (verbose && (error instanceof PlapiError || error instanceof FapiError) && error.url) {
10621052
log.error(` URL: ${error.url}`);
10631053
}
1054+
if (verbose && error.clerkTraceId) {
1055+
log.error(` Trace: ${error.clerkTraceId}`);
1056+
}
10641057
}
10651058
process.exit(EXIT_CODE.GENERAL);
10661059
}
@@ -1110,32 +1103,3 @@ function outputJsonError(
11101103
if (errors?.length) payload.error.errors = errors;
11111104
log.raw(JSON.stringify(payload));
11121105
}
1113-
1114-
/** Extract the error code from a Clerk API JSON response body, if present. */
1115-
function extractApiErrorCode(body: string): string | undefined {
1116-
try {
1117-
const parsed = JSON.parse(body);
1118-
return parsed.errors?.[0]?.code;
1119-
} catch {
1120-
return undefined;
1121-
}
1122-
}
1123-
1124-
/** Extract the full errors array from a Clerk API JSON response body, if present. */
1125-
function extractApiErrors(body: string): ApiErrorEntry[] | undefined {
1126-
try {
1127-
const parsed = JSON.parse(body);
1128-
if (Array.isArray(parsed.errors) && parsed.errors.length > 0) {
1129-
return parsed.errors.map((e: ApiErrorEntry) => {
1130-
const entry: ApiErrorEntry = {};
1131-
if (e.code) entry.code = e.code;
1132-
if (e.message) entry.message = e.message;
1133-
if (e.meta && Object.keys(e.meta).length > 0) entry.meta = e.meta;
1134-
return entry;
1135-
});
1136-
}
1137-
} catch {
1138-
// not JSON
1139-
}
1140-
return undefined;
1141-
}

0 commit comments

Comments
 (0)