refactor: simplify Prismic client and error handling internals#225
refactor: simplify Prismic client and error handling internals#225angeloashmore wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ff6c53c. Configure here.
| targetFields[fieldId] = field; | ||
| await saveModel(); | ||
| addField(fields, fieldId, field); | ||
| await save(); |
There was a problem hiding this comment.
Field add hints removed
Low Severity
Every prismic field add command now logs only Field added: … and no longer prints the follow-up guidance that getPostFieldAddMessage used to emit after a successful add, despite the PR stating no intended user-facing behavior change.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ff6c53c. Configure here.
There was a problem hiding this comment.
It's fine. It wasn't really useful.
There was a problem hiding this comment.
Vaguely related, but have you considered adding evals for the CLI (in future work)? Since it's partly (majoritarily?) intended to be used by AI, it might be nice to catch potential regressions related to messaging/API usage programmatically.
I like @vercel/agent-eval DX in that regard (I think I already shared: https://github.com/vercel-labs/agent-eval), maybe you'll prefer something smaller/not remote sandboxed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read response bodies once, add explicit json/body request options, and move not-found messaging into request(). Standardize every client on bare-base get*ServiceUrl builders and *ServiceRequest transport helpers, with repository scoping handled in the helpers. Remove the unused amplitude client and pass-through request helpers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an endpoint-level `unknownErrorMessage` request option so client functions declare their failure message once, instead of each command wrapping calls in a try/catch that converts `UnknownRequestError` into a `CommandError`. Removes 35 duplicated try/catch blocks across 26 command files. The message now lives on the endpoint, so it applies to every caller (including previously bare calls to shared endpoints like getProfile and the preview endpoints). The server response body is no longer appended. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getToken() and getHost() were never called apart — 34 command, adapter, and index.ts sites called them as a pair — and each independently read credentials.json, so every command read the file from disk twice. Replace both with one getCredentials() that reads the file once and applies the same env-var precedence and default-host fallback. Call sites collapse from two awaited lines to one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Drop the unused issues field and custom constructors from the config Invalid* error classes; nothing ever read them, so the read functions' catch blocks lose their ZodError branching. - updateRoute delegates straight to addRoute/removeRoute (the page branch duplicated the read and hasRoute check addRoute already does). - checkIsTypeScriptProject returns exists() directly. - Unexport symbols only referenced inside the module (findConfigPath, findSuggestedConfigPath, findLegacySliceMachineConfigPath, MissingLegacySliceMachineConfigError, the Route type). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
removeLocale inherited the client-level "Repository not found" default, so removing a non-existent locale reported the repository as missing. Override with "Locale not found: <code>", matching the resource-specific messages getCustomType/getSlice already use. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
deleteSlice/deleteCustomType already look up the resource by id and throw the same not-found error, so the explicit get before delete did no extra work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Field add, edit, and reorder now surface duplicate and missing-field errors using the full dotted path the user passed instead of only the leaf segment. Update and remove requests for a specific type or slice now report a model-not-found message on 404 instead of the generic repository-not- found message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
64a6f2c to
5ae52a8
Compare
lihbr
left a comment
There was a problem hiding this comment.
LGTM, super rework!
Commands are much cleaner to read without try..catch or error throwing. I also like the new API used to manipulate models.
| targetFields[fieldId] = field; | ||
| await saveModel(); | ||
| addField(fields, fieldId, field); | ||
| await save(); |
There was a problem hiding this comment.
Vaguely related, but have you considered adding evals for the CLI (in future work)? Since it's partly (majoritarily?) intended to be used by AI, it might be nice to catch potential regressions related to messaging/API usage programmatically.
I like @vercel/agent-eval DX in that regard (I think I already shared: https://github.com/vercel-labs/agent-eval), maybe you'll prefer something smaller/not remote sandboxed.
| if (!UNTRACKED_COMMANDS.includes(command)) { | ||
| trackCommandEnd(command); | ||
| } |
There was a problem hiding this comment.
💡 #idea: This bit is repeated on every case (some also track the error itself), maybe there's a way to abstract it before/after all the cases?


Resolves:
Description
Internal cleanup of the CLI. No user-facing behavior change is intended.
Reorganizes the Prismic client internals, collapses the auth token/host accessors into one, and moves per-command error wrapping to a single request-level option so error messages live next to the endpoints that produce them.
Checklist
Preview
How to QA
Note
Medium Risk
Large import-path and error-handling refactor across most commands and API clients; mistakes could change CLI error text or break endpoints, though behavior is intended to stay the same.
Overview
Internal refactor of the Prismic CLI with no intended user-facing behavior change. HTTP clients move from
src/clients/*tosrc/lib/prismic/clients/*, with shared helpers per service andrequestoptions likenotFoundMessage/unknownErrorMessageso commands no longer wrap every API call inUnknownRequestErrorhandlers.Auth replaces separate
getToken()/getHost()withgetCredentials()returning both values; adapters and commands are updated accordingly.Field and model logic moves out of
src/modelsintosrc/fields.ts(CLI targets/sources,getNewFieldTarget, reorder helpers) andsrc/lib/prismic/models(addField,reorderField, typed errors). The CLI entrypoint maps those model errors to user messages. Field add commands drop post-add hint output;field-reorderusesexactlyOneOptionfor--before/--after.Smaller cleanups:
environmentsunderlib/prismic,pushinlines dashboard URL helpers removed fromwroom,type-remove/slice-removedelete by id without re-fetching the model, and the oldamplitudeclient file is removed from this tree.Reviewed by Cursor Bugbot for commit 5ae52a8. Bugbot is set up for automated code reviews on this repo. Configure here.