Skip to content

Commit 337e796

Browse files
wyattjohwyattjoh
authored
refactor(errors): expose structured Clerk error fields on ApiError subclasses (#288)
* refactor(errors): parse Clerk error envelope into structured ApiError fields * feat(errors): add fromResponse factories to ApiError subclasses * refactor(plapi): construct PlapiError via fromResponse * refactor(fapi): construct FapiError via fromResponse * refactor(keyless): construct BapiError via fromResponse * refactor(bapi): construct BapiError via fromResponse * refactor(users): construct BapiError via fromResponse * test: construct ApiError subclasses via fromBody in fixtures * 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. * fix(errors): guard parseApiBody against null/non-object JSON bodies JSON.parse("null") and other non-object JSON values caused parseApiBody to throw a TypeError when accessing envelope.errors. Add an early fallback so the global error handler shows a truncated body instead of crashing.
1 parent b581d5f commit 337e796

18 files changed

Lines changed: 330 additions & 114 deletions

‎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-
}

‎packages/cli-core/src/commands/api/bapi.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,12 @@ export async function bapiRequest(options: {
4343
body: options.body,
4444
});
4545

46-
const rawBody = await response.text();
47-
4846
if (!response.ok) {
49-
throw new BapiError(response.status, rawBody, response.headers);
47+
throw await BapiError.fromResponse(response);
5048
}
5149

50+
const rawBody = await response.text();
51+
5252
let body: unknown;
5353
try {
5454
body = JSON.parse(rawBody);

‎packages/cli-core/src/commands/config/pull.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,6 @@ describe("config pull", () => {
310310
instances: { development: "ins_dev" },
311311
});
312312

313-
await expect(runConfigPull()).rejects.toThrow("API error");
313+
await expect(runConfigPull()).rejects.toThrow("Unauthorized");
314314
});
315315
});

‎packages/cli-core/src/commands/config/push.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ describe("config push", () => {
632632
instances: { development: "ins_dev" },
633633
});
634634

635-
await expect(runConfigPatch({ json: '{"a":1}', yes: true })).rejects.toThrow("API error");
635+
await expect(runConfigPatch({ json: '{"a":1}', yes: true })).rejects.toThrow("Bad Request");
636636
});
637637

638638
test("shows success message after push", async () => {

‎packages/cli-core/src/commands/config/schema.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,6 @@ describe("config schema", () => {
275275
instances: { development: "ins_dev" },
276276
});
277277

278-
await expect(runConfigSchema()).rejects.toThrow("API error");
278+
await expect(runConfigSchema()).rejects.toThrow("Unauthorized");
279279
});
280280
});

‎packages/cli-core/src/commands/doctor/doctor.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ describe("checkLinkedAppExists", () => {
346346
const ctx = createMockContext({
347347
token: "test_token",
348348
profile: mockProfile,
349-
applicationError: new PlapiError(404, "Not found"),
349+
applicationError: PlapiError.fromBody(404, "Not found"),
350350
});
351351
const result = await checkLinkedAppExists(ctx);
352352
expectCheck(result, {

‎packages/cli-core/src/commands/env/pull.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ describe("env pull", () => {
487487
instances: { development: "ins_dev" },
488488
});
489489

490-
await expect(runEnvPull()).rejects.toThrow("API error");
490+
await expect(runEnvPull()).rejects.toThrow("Unauthorized");
491491
});
492492

493493
test("sends include_secret_keys=true in API request", async () => {

‎packages/cli-core/src/commands/link/index.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ describe("link", () => {
587587
test("shows picker with only create option when listApplications fails with 500", async () => {
588588
mockIsAgent.mockReturnValue(false);
589589
mockGetToken.mockResolvedValue("token");
590-
mockListApplications.mockRejectedValue(new PlapiError(500, "Internal Server Error"));
590+
mockListApplications.mockRejectedValue(PlapiError.fromBody(500, "Internal Server Error"));
591591
mockSearch.mockImplementation(
592592
async (config: {
593593
source: (term: string | undefined) => { name: string; value: string }[];
@@ -622,7 +622,7 @@ describe("link", () => {
622622
test("propagates listApplications errors that are not 5xx", async () => {
623623
mockIsAgent.mockReturnValue(false);
624624
mockGetToken.mockResolvedValue("token");
625-
mockListApplications.mockRejectedValue(new PlapiError(401, "Unauthorized"));
625+
mockListApplications.mockRejectedValue(PlapiError.fromBody(401, "Unauthorized"));
626626

627627
await expect(runLink()).rejects.toBeInstanceOf(PlapiError);
628628
});
@@ -1248,7 +1248,7 @@ describe("link", () => {
12481248
mockListApplications.mockResolvedValue([mockApp]);
12491249
mockSearch.mockResolvedValue("__create_new__");
12501250
mockInput.mockResolvedValue("My App");
1251-
mockCreateApplication.mockRejectedValue(new PlapiError(422, "Unprocessable Entity"));
1251+
mockCreateApplication.mockRejectedValue(PlapiError.fromBody(422, "Unprocessable Entity"));
12521252

12531253
await expect(link()).rejects.toBeInstanceOf(PlapiError);
12541254
expect(mockSetProfile).not.toHaveBeenCalled();
@@ -1265,7 +1265,7 @@ describe("link", () => {
12651265
name: "My App",
12661266
instances: [],
12671267
});
1268-
mockFetchApplication.mockRejectedValue(new PlapiError(503, "Service Unavailable"));
1268+
mockFetchApplication.mockRejectedValue(PlapiError.fromBody(503, "Service Unavailable"));
12691269

12701270
await expect(link()).rejects.toBeInstanceOf(PlapiError);
12711271
expect(mockSetProfile).not.toHaveBeenCalled();

0 commit comments

Comments
 (0)