refactor(errors): expose structured Clerk error fields on ApiError subclasses#288
Merged
Conversation
Contributor
Author
|
Stack: api-errors-and-deploy Part of a stacked-prs chain. Do not merge manually. |
This was referenced May 13, 2026
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR centralizes parsing of Clerk-style API error envelopes and exposes structured fields on ApiError (code, longMessage, meta, clerkTraceId). It adds fromBody/fromResponse factories for PlapiError, FapiError, and BapiError, updates API clients to throw factory-built errors (deferring body reads until after status checks), rewrites CLI formatApiBody to accept ApiError and format structured or verbose output, and updates tests/mocks to use the new factories and adjust expected messages. 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
wyattjoh
changed the title
wyattjoh
force-pushed
the
wyattjoh/api-error-classification
branch
2 times, most recently
from
bf05f3e to
ffb869f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/cli-core/src/lib/errors.ts (1)
200-202:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winHandle
nullJSON bodies before accessing envelope properties.If the response body is the JSON string
"null",JSON.parsesucceeds butparsedisnull. Line 201'senvelope.errors?.[0]will throw aTypeErrorwhen trying to access.errorsonnull(the optional chaining?.[0]doesn't protect the initial.errorsaccess).Proposed fix
+ if (parsed === null || typeof parsed !== "object") { + return fallback(truncateBody(body)); + } + const envelope = parsed as ClerkErrorEnvelope; - const first = envelope.errors?.[0]; + const first = Array.isArray(envelope.errors) ? envelope.errors[0] : undefined;🤖 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 assumes JSON.parse returned an object and does const envelope = parsed as ClerkErrorEnvelope and then accesses envelope.errors?.[0], which will throw if parsed is null; update the logic to first check parsed !== null (or parsed == null) before casting/using envelope, e.g. treat parsed as ClerkErrorEnvelope | null and if parsed == null handle the empty/null body case (return a sensible default or error) or use a guarded access like if (parsed && (parsed as ClerkErrorEnvelope).errors) to safely compute first; adjust any downstream uses of envelope/first accordingly.
🤖 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.
Duplicate comments:
In `@packages/cli-core/src/lib/errors.ts`:
- Around line 200-202: The code assumes JSON.parse returned an object and does
const envelope = parsed as ClerkErrorEnvelope and then accesses
envelope.errors?.[0], which will throw if parsed is null; update the logic to
first check parsed !== null (or parsed == null) before casting/using envelope,
e.g. treat parsed as ClerkErrorEnvelope | null and if parsed == null handle the
empty/null body case (return a sensible default or error) or use a guarded
access like if (parsed && (parsed as ClerkErrorEnvelope).errors) to safely
compute first; adjust any downstream uses of envelope/first accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4efb2f17-e7bc-4f87-8a22-77dced2a9d78
📒 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
✅ Files skipped from review due to trivial changes (2)
- packages/cli-core/src/commands/env/pull.test.ts
- packages/cli-core/src/commands/config/schema.test.ts
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.
wyattjoh
force-pushed
the
wyattjoh/api-error-classification
branch
from
ffb869f to
162f269
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/cli-core/src/lib/errors.ts (1)
200-201:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard null parsed JSON before reading
errors.At Line 200 and Line 201,
JSON.parse(body)can returnnull(e.g. body is"null"), andenvelope.errorswill throw aTypeError, skipping 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;🤖 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 - 201, parsed can be null if JSON.parse(body) returns null, causing envelope.errors access to throw; update the logic around parsed/envelope (the parsed variable, ClerkErrorEnvelope cast and the first = envelope.errors?.[0] line) to guard against null/non-object parsed values by checking parsed !== null && typeof parsed === 'object' (or using a safe fallback) before casting or accessing envelope.errors, and fall back to the existing generic error handling when the check fails.
🤖 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.
Duplicate comments:
In `@packages/cli-core/src/lib/errors.ts`:
- Around line 200-201: parsed can be null if JSON.parse(body) returns null,
causing envelope.errors access to throw; update the logic around parsed/envelope
(the parsed variable, ClerkErrorEnvelope cast and the first =
envelope.errors?.[0] line) to guard against null/non-object parsed values by
checking parsed !== null && typeof parsed === 'object' (or using a safe
fallback) before casting or accessing envelope.errors, and fall back to the
existing generic error handling when the check fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6b636d0-ed30-448f-95dd-6489e0483618
📒 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
✅ Files skipped from review due to trivial changes (2)
- packages/cli-core/src/commands/doctor/doctor.test.ts
- packages/cli-core/src/commands/link/index.test.ts
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.
rafa-thayto
approved these changes
May 21, 2026
rafa-thayto
approved these changes
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.The parser also guards against JSON bodies whose top level is
nullor another non-object value, so a response body of"null"no longer crashes the global handler with aTypeErrorand falls back to the truncated-body path instead.Test plan
bun run typecheckbun run test