Conversation
MichaelUnkey
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughUpdated the TRPC mutation error handler in use-root-key-dialog.ts to switch from a typed error message display to conditional handling based on err.data?.code, specifically showing a BAD_REQUEST-specific message and a generic fallback. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as RootKeyDialog
participant S as TRPC rootKey.create
U->>D: Submit create root key
D->>S: mutate(data)
alt Success
S-->>D: result
D-->>U: Close dialog / success UI
else Error
S-->>D: err
alt err.data?.code == "BAD_REQUEST"
Note over D: Show "You need to add at least one permission."
else Other error
Note over D: Show "Something went wrong. Please try again."
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/app/(app)/settings/root-keys/components/root-key/hooks/use-root-key-dialog.ts (3)
160-165: Add a local guard to prevent invalid create calls (defense-in-depth).UI disables the button, but add a runtime check to avoid server roundtrips if the action is triggered programmatically.
} else { // Create new key - key.mutate({ + if (selectedPermissions.length === 0) { + toast.error("You need to add at least one permission."); + return; + } + key.mutate({ name: name && name.length > 0 ? name : undefined, permissions: selectedPermissions, }); }
103-106: Avoid exposing raw server error messages in updateName.For consistency with the create flow and to prevent leaking internal error text, switch to code-based handling (or reuse a shared error handler).
- onError(err: { message: string }) { - toast.error(err.message); - }, + onError(err: unknown) { + if (err instanceof (await import("@trpc/client")).TRPCClientError) { + switch (err.data?.code) { + case "FORBIDDEN": + case "UNAUTHORIZED": + toast.error("You don’t have permission to update this key."); + break; + default: + toast.error("Something went wrong. Please try again."); + } + } else { + toast.error("Something went wrong. Please try again."); + } + },
114-116: Unify error handling in updatePermissions with the new pattern.Same rationale as above; keep error UX consistent across mutations.
- onError(err: { message: string }) { - toast.error(err.message); - }, + onError(err: unknown) { + if (err instanceof (await import("@trpc/client")).TRPCClientError) { + switch (err.data?.code) { + case "FORBIDDEN": + case "UNAUTHORIZED": + toast.error("You don’t have permission to update permissions."); + break; + default: + toast.error("Something went wrong. Please try again."); + } + } else { + toast.error("Something went wrong. Please try again."); + } + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/dashboard/app/(app)/settings/root-keys/components/root-key/hooks/use-root-key-dialog.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/dashboard/app/(app)/settings/root-keys/components/root-key/hooks/use-root-key-dialog.ts (1)
196-200: Button enablement logic matches PR intent.Enabling create only when there are permissions aligns with “don’t show the standard text error by default.”
| onError(err) { | ||
| if (err.data?.code === "BAD_REQUEST") { | ||
| toast.error("You need to add at least one permission."); | ||
| } else { | ||
| toast.error("Something went wrong. Please try again."); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type the TRPC error and handle known codes explicitly (avoid implicit any and improve UX).
onError(err) is untyped and relies on .data?.code existing. Tighten types and provide explicit handling for common codes to prevent accidental message leakage and improve clarity.
@@
- const key = trpc.rootKey.create.useMutation({
+ const key = trpc.rootKey.create.useMutation({
onSuccess() {
toast.success(ROOT_KEY_MESSAGES.SUCCESS.ROOT_KEY_GENERATED);
trpcUtils.settings.rootKeys.query.invalidate();
},
- onError(err) {
- if (err.data?.code === "BAD_REQUEST") {
- toast.error("You need to add at least one permission.");
- } else {
- toast.error("Something went wrong. Please try again.");
- }
- },
+ onError(err: unknown) {
+ // Avoid showing raw server messages; branch on safe, known codes only.
+ if (err instanceof (await import("@trpc/client")).TRPCClientError) {
+ switch (err.data?.code) {
+ case "BAD_REQUEST":
+ toast.error("You need to add at least one permission.");
+ break;
+ case "FORBIDDEN":
+ case "UNAUTHORIZED":
+ toast.error("You don’t have permission to create a root key.");
+ break;
+ default:
+ toast.error("Something went wrong. Please try again.");
+ }
+ } else {
+ toast.error("Something went wrong. Please try again.");
+ }
+ },
});Committable suggestion skipped: line range outside the PR's diff.
chronark
deleted the
eng-1742-improve-error-response-for-root-key-creation-failure
branch
2 participants
What does this PR do?
Fixes # (issue)
ENG-1742
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit