refactor(errors): expose structured Clerk error fields on ApiError subclasses#288
refactor(errors): expose structured Clerk error fields on ApiError subclasses#288wyattjoh wants to merge 9 commits into
Conversation
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.
|
Stack: api-errors-and-deploy Part of a stacked PR chain. Do not merge manually. |
|
📝 WalkthroughWalkthroughThis pull request refactors HTTP API error handling across the CLI by introducing factory methods and structured field extraction. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli-core/src/lib/errors.ts`:
- Around line 200-202: The code currently casts parsed to ClerkErrorEnvelope and
reads envelope.errors?.[0] which will throw if the API returned JSON null; add a
guard that checks parsed == null (or parsed === null) before casting/reading so
you early-fallback instead of accessing properties on null. Concretely, update
the logic around the variables parsed, envelope and first (the lines creating
const envelope = parsed as ClerkErrorEnvelope; const first =
envelope.errors?.[0];) to first check if parsed == null and handle the
fallback/error path, or coerce envelope to a safe object (e.g., const envelope =
parsed as ClerkErrorEnvelope | null; if (!parsed || !envelope?.errors?.[0]) {
... }) so you never access .errors on a null parsed value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da47d176-356e-4745-8664-aaf56f760dac
📒 Files selected for processing (18)
packages/cli-core/src/cli-program.test.tspackages/cli-core/src/cli-program.tspackages/cli-core/src/commands/api/bapi.tspackages/cli-core/src/commands/config/pull.test.tspackages/cli-core/src/commands/config/push.test.tspackages/cli-core/src/commands/config/schema.test.tspackages/cli-core/src/commands/doctor/doctor.test.tspackages/cli-core/src/commands/env/pull.test.tspackages/cli-core/src/commands/link/index.test.tspackages/cli-core/src/commands/users/create.test.tspackages/cli-core/src/commands/users/interactive/instance-context.tspackages/cli-core/src/lib/bapi-command.test.tspackages/cli-core/src/lib/errors.test.tspackages/cli-core/src/lib/errors.tspackages/cli-core/src/lib/fapi.tspackages/cli-core/src/lib/keyless.tspackages/cli-core/src/lib/plapi.tspackages/cli-core/src/lib/token-exchange.test.ts
| const envelope = parsed as ClerkErrorEnvelope; | ||
| const first = envelope.errors?.[0]; | ||
| if (!first) { |
There was a problem hiding this comment.
Handle null JSON bodies before envelope access.
At Line 200, parsed is cast and then envelope.errors?.[0] is accessed. If the API returns JSON null, this throws a TypeError and bypasses your fallback path.
Proposed fix
- const envelope = parsed as ClerkErrorEnvelope;
- const first = envelope.errors?.[0];
+ if (parsed === null || typeof parsed !== "object") {
+ return fallback(truncateBody(body));
+ }
+
+ const envelope = parsed as ClerkErrorEnvelope;
+ const first = Array.isArray(envelope.errors) ? envelope.errors[0] : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const envelope = parsed as ClerkErrorEnvelope; | |
| const first = envelope.errors?.[0]; | |
| if (!first) { | |
| if (parsed === null || typeof parsed !== "object") { | |
| return fallback(truncateBody(body)); | |
| } | |
| const envelope = parsed as ClerkErrorEnvelope; | |
| const first = Array.isArray(envelope.errors) ? envelope.errors[0] : undefined; | |
| if (!first) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli-core/src/lib/errors.ts` around lines 200 - 202, The code
currently casts parsed to ClerkErrorEnvelope and reads envelope.errors?.[0]
which will throw if the API returned JSON null; add a guard that checks parsed
== null (or parsed === null) before casting/reading so you early-fallback
instead of accessing properties on null. Concretely, update the logic around the
variables parsed, envelope and first (the lines creating const envelope = parsed
as ClerkErrorEnvelope; const first = envelope.errors?.[0];) to first check if
parsed == null and handle the fallback/error path, or coerce envelope to a safe
object (e.g., const envelope = parsed as ClerkErrorEnvelope | null; if (!parsed
|| !envelope?.errors?.[0]) { ... }) so you never access .errors on a null parsed
value.
wyattjoh
changed the title
1 participant
Summary
ApiErrorand itsPlapiError/FapiError/BapiErrorsubclasses now parse the Clerk error envelope at construction time and exposecode,long_message,meta, andclerk_trace_idas typed instance fields. The base constructor uses the parsedmessage(or a short truncated fallback for non-JSON bodies) instead of the previousAPI error (<status>): <raw body>string, so defaulterror.messagerendering already shows the human-readable Clerk message. The global error handler incli-program.tsreads those fields directly rather than re-parsing the body, which collapses the existing ad-hoc body-string branching.Each subclass gains a
fromResponse(response: Response)async factory that reads the body once and constructs the error, replacing the manualresponse.text()+new XError(...)pattern at every PLAPI / FAPI / KEYLESS / BAPI / users call site. The existingfromBodyhelpers stay for tests and fixtures that already have the body string in hand.Test plan
bun run typecheckbun run test