fix(keys): removeKey 错误码透传 + 密钥创建弹窗文案与真实行为对齐#1266
Conversation
The removeKey action previously returned hardcoded Chinese strings or generic messages, causing the UI to display unhelpful toasts via the REST bridge. It now returns specific error codes that the UI translates through the errors namespace. Additionally, the key creation dialogs incorrectly claimed the full key is only shown once. The copy now accurately reflects that owners and admins can reveal and copy the full key from the key list at any time. Includes regression tests for the action error codes, API error mapping, UI toast translation, and i18n copy guards.
📝 Walkthrough总体概述这个 PR 改进了密钥删除的错误处理机制。将密钥创建成功后的文案从"仅显示一次"改为"可随时重新查看"，同时新增两个业务错误码（不能删除最后一个密钥、删除后无可用分组），在后端验证逻辑中返回对应的错误码，前端根据错误码翻译后展示给用户，并通过完整的测试覆盖验证各个环节。 变更说明多语言文案与错误码更新
🎯 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates API key deletion error handling and refactors key creation copy across multiple locales to accurately reflect that keys can be viewed again. It introduces specific error codes (such as CANNOT_DELETE_LAST_KEY and CANNOT_DELETE_LAST_GROUP_KEY) and updates the backend action, API mapping, and frontend UI to display localized error messages instead of generic errors. Comprehensive unit tests have also been added. The review feedback suggests hoisting the tError initialization outside of the try block in removeKey to avoid duplicate translation fetching in the catch block.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export async function removeKey(keyId: number): Promise<ActionResult> { | ||
| try { | ||
| const tError = await getTranslations("errors"); |
There was a problem hiding this comment.
建议将 tError 的获取提升到 try 块外部。这样 tError 变量在 try 和 catch 块中都处于同一作用域内，从而避免在 catch 块中重复调用 getTranslations("errors") 并重新声明该变量，提升代码执行效率和可维护性。
| export async function removeKey(keyId: number): Promise<ActionResult> { | |
| try { | |
| const tError = await getTranslations("errors"); | |
| export async function removeKey(keyId: number): Promise<ActionResult> { | |
| const tError = await getTranslations("errors"); | |
| try { |
| const tError = await getTranslations("errors"); | ||
| const message = error instanceof Error ? error.message : tError("DELETE_KEY_FAILED"); |
There was a problem hiding this comment.
配合将 tError 提升到 try 块外部的修改，这里可以移除重复的 getTranslations 调用和变量声明，直接使用外层作用域的 tError。
| const tError = await getTranslations("errors"); | |
| const message = error instanceof Error ? error.message : tError("DELETE_KEY_FAILED"); | |
| const message = error instanceof Error ? error.message : tError("DELETE_KEY_FAILED"); |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -780,7 +796,7 @@ export async function removeKey(keyId: number): Promise<ActionResult> { | |||
| errorMessage: "DELETE_FAILED", | |||
| redactExtraKeys: ["key"], | |||
| }); | |||
| return { ok: false, error: message }; | |||
| return { ok: false, error: message, errorCode: ERROR_CODES.DELETE_FAILED }; | |||
There was a problem hiding this comment.
When the catch block fires with a real Error, error.message (e.g. a raw DB driver message) is placed in the error field and then serialised into the REST response body. The dashboard UI now silently ignores that field (it switches to the DELETE_FAILED translation as soon as errorCode is present), but any REST API consumer that inspects the response body still sees the internal message. Consider using the sanitised translation even for Error instances in the catch path so the error field is safe to expose: const message = tError("DELETE_KEY_FAILED");.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/keys.ts
Line: 786-799
Comment:
**Raw exception message leaks into REST response body**
When the catch block fires with a real `Error`, `error.message` (e.g. a raw DB driver message) is placed in the `error` field and then serialised into the REST response body. The dashboard UI now silently ignores that field (it switches to the `DELETE_FAILED` translation as soon as `errorCode` is present), but any REST API consumer that inspects the response body still sees the internal message. Consider using the sanitised translation even for `Error` instances in the catch path so the `error` field is safe to expose: `const message = tError("DELETE_KEY_FAILED");`.
How can I resolve this? If you propose a fix, please make it concise.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/actions/keys.ts (1)
788-789: ⚡ Quick win消除冗余的翻译获取调用。
第 788 行在 catch 块中重新调用
getTranslations("errors")，但外层作用域已在第 689 行声明了tError。由于 try 块从第 688 行开始，当错误发生在第 689 行之后时（大多数情况），外层的tError已经可用，重复获取翻译是低效的。建议直接使用外层的
tError或添加条件判断。改进建议
} catch (error) { logger.error("删除密钥失败:", error); - const tError = await getTranslations("errors"); - const message = error instanceof Error ? error.message : tError("DELETE_KEY_FAILED"); + const message = error instanceof Error ? error.message : tError("DELETE_KEY_FAILED"); emitActionAudit({ category: "key", action: "key.delete", targetType: "key", targetId: String(keyId), success: false, errorMessage: "DELETE_FAILED", redactExtraKeys: ["key"], }); return { ok: false, error: message, errorCode: ERROR_CODES.DELETE_FAILED }; }🤖 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 `@src/actions/keys.ts` around lines 788 - 789, The catch block redundantly calls getTranslations("errors") again even though tError was already declared in the outer scope; update the catch handling in the function that uses tError so it reuses the outer tError (or only calls getTranslations if tError is undefined) instead of always invoking getTranslations("errors") again, and ensure the error message assignment uses the existing tError variable (and keeps the fallback to error.message when error instanceof Error).
🤖 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.
Nitpick comments:
In `@src/actions/keys.ts`:
- Around line 788-789: The catch block redundantly calls
getTranslations("errors") again even though tError was already declared in the
outer scope; update the catch handling in the function that uses tError so it
reuses the outer tError (or only calls getTranslations if tError is undefined)
instead of always invoking getTranslations("errors") again, and ensure the error
message assignment uses the existing tError variable (and keeps the fallback to
error.message when error instanceof Error).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9866cda1-89ae-4a70-9c2b-19a931e03604
📒 Files selected for processing (18)
messages/en/dashboard.jsonmessages/en/errors.jsonmessages/ja/dashboard.jsonmessages/ja/errors.jsonmessages/ru/dashboard.jsonmessages/ru/errors.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/errors.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/errors.jsonsrc/actions/keys.tssrc/app/[locale]/dashboard/_components/user/forms/delete-key-confirm.tsxsrc/app/[locale]/dashboard/_components/user/user-key-table-row.tsxsrc/lib/utils/error-messages.tstests/unit/actions/keys-remove-error-codes.test.tstests/unit/api/v1/keys-delete-error-mapping.test.tstests/unit/delete-key-error-toast-ui.test.tsxtests/unit/i18n/key-created-copy.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR cleanly addresses two related issues: (1) removeKey error paths lacked errorCode, causing the REST bridge to return generic "Bad request" details instead of the real reason; (2) key-creation dialog copy incorrectly claimed the full key could "only be shown once" despite the reveal feature existing. Both fixes are well-scoped with end-to-end coverage across the action layer, REST handler, UI toast, and i18n files.
PR Size: L
- Lines changed: 588 (558 additions, 30 deletions)
- Files changed: 18
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Notes
The PR is well-executed. Specific observations that passed validation:
- Error code propagation chain: All 6 error paths in
removeKeynow includeerrorCodewith proper i18n messages. The REST handler'sactionError()correctly mapsNOT_FOUNDto 404,UNAUTHORIZED/PERMISSION_DENIEDto 403, and business codes to 400. - i18n completeness: Both new error codes (
CANNOT_DELETE_LAST_KEY,CANNOT_DELETE_LAST_GROUP_KEY) have translations in all 5 locales. All pre-existing translation keys used by the action (KEY_NOT_FOUND,UNAUTHORIZED,PERMISSION_DENIED,DELETE_KEY_FAILED) are confirmed present. - UI consistency: Both delete entry points (
delete-key-confirm.tsx,user-key-table-row.tsx) follow the sameerrorCode-first pattern already established inadd-key-form.tsx, with proper fallback to raw error for backward compatibility. - Test coverage: 4 new test files provide 7 action tests, 7 handler mapping tests, 2 UI toast tests, and 5-locale i18n regression tests. Coverage is comprehensive.
- Pre-existing note: The catch block in
removeKeystill passeserror.messageinto theerrorfield forErrorinstances, which can leak internal DB messages into the REST response body. However, this is pre-existing (unchanged in this PR) and the newerrorCodefield ensures the UI uses the sanitized translation instead.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Comprehensive (4 new test files)
- Code clarity - Good
Automated review by Claude AI
评审复核(deep-review + /code-review max effort)本轮多代理评审已覆盖本 PR,未发现需要改动的代码问题:removeKey 六条错误路径的 errorCode 透传、五语言 errors.json 文案、两处删除入口的 errorCode 翻译均经独立验证确认正确;弹窗文案"可随时重新查看"与真实 reveal 行为一致(reveal 鉴权 owner/admin 未变)。 唯一的红色检查 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
问题
Fixes #1256
1. removeKey 错误返回缺少 errorCode，真实错误被吞掉
src/actions/keys.ts的removeKey所有错误返回都没有设置errorCode字段，而 keys handler 的actionError()在errorCode为 undefined 时兜底为 400 +key.action_failed，且detail被publicActionErrorDetail()置为通用文案（"Bad request"）。REST 桥接路径上客户端 toast 只显示res.error（即通用 detail），导致"该用户至少需要保留一个可用的密钥"等真实原因完全无法透出。2. 密钥创建弹窗宣称"完整密钥仅显示一次"，与实际行为不符
创建密钥后的提示称"完整密钥仅显示一次，请立即复制并妥善保存"/"关闭后将无法再次查看此密钥"，但实际上 owner 和 admin 随时可以通过密钥列表的 reveal 功能（
getUnmaskedKey/GET /api/v1/keys/{id}:reveal）重新查看并复制完整密钥。修复
removeKey 错误码（含全链路透传）
removeKey全部 6 个错误路径补齐errorCode并将硬编码中文消息 i18n 化：UNAUTHORIZED（403）NOT_FOUND（404）PERMISSION_DENIED（403）CANNOT_DELETE_LAST_KEY（400）CANNOT_DELETE_LAST_GROUP_KEY（400）DELETE_FAILED（400）ERROR_CODES新增 2 个业务码，5 语言errors.json补齐对应翻译user-key-table-row.tsx/delete-key-confirm.tsx）改为优先按errorCode走 errors 命名空间翻译（与add-key-form.tsx既有模式一致），无 errorCode 时保留原 fallback创建弹窗文案（5 语言 × 4 处）
keyListHeader.keyCreatedDialog.description/warningTextaddKeyForm.generatedKey.hintuserManagement.createDialog.keyHint统一改为：可在列表中管理并随时重新查看完整密钥，请妥善保管避免泄露。遵循 ja 半角括号、zh-TW 全角括号的既有规则。
测试（TDD，先红后绿）
tests/unit/actions/keys-remove-error-codes.test.ts：7 个用例覆盖全部错误路径的 errorCode + i18n 消息及成功路径tests/unit/api/v1/keys-delete-error-mapping.test.ts：DELETE handler 对各 errorCode 的 404/403/400 映射与透传契约tests/unit/delete-key-error-toast-ui.test.tsx：UI 按 errorCode 翻译 toast、无 errorCode 时回退原始消息tests/unit/i18n/key-created-copy.test.ts：5 语言守护——禁止"仅显示一次"类表述、必须包含"可重新查看"表述、新错误码翻译存在且具体验证
bun run build/lint/typecheck/test（根配置全量）/test:v1全部通过test:integration/test:e2e通过（无 DSN 自动跳过 DB 用例）🤖 Generated with Claude Code
Greptile Summary
This PR fixes two distinct issues:
removeKeyaction errors were silently swallowed (noerrorCodeon any return path, so clients only ever saw a generic "Bad request" detail), and the key-creation dialogs incorrectly told users the full key could never be seen again when owners/admins can re-reveal it via the key list at any time.removeKeynow include a structurederrorCode; hard-coded Chinese messages are replaced with i18n lookups; two new business codes (CANNOT_DELETE_LAST_KEY,CANNOT_DELETE_LAST_GROUP_KEY) are added toBUSINESS_ERRORSand translated across all 5 locales.delete-key-confirm.tsx,user-key-table-row.tsx) now prioritiseerrorCodefor toast messages, matching the existing pattern inadd-key-form.tsx.Confidence Score: 4/5
Safe to merge — changes are well-scoped to error handling and copy, with comprehensive unit test coverage across all 6 error paths.
The error-code propagation chain is correctly implemented end-to-end (action → REST bridge → UI toast), translations are present in all 5 locales, and the new codes are guarded by regression tests. One leftover: the catch block still places the raw
error.messagein theerrorfield even though the UI now ignores it, meaning a DB-level exception message is still serialised into the REST response body.The catch block in
src/actions/keys.ts(around line 789) is the only spot worth a second look — theerrorfield path vs. theerrorCodedisplay path now diverge for exception cases.Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as Dashboard UI participant Bridge as REST Bridge participant Action as removeKey participant DB as Repository UI->>Bridge: "DELETE /api/v1/keys/{id}" Bridge->>Action: removeKey(keyId) alt No session Action-->>Bridge: ok:false, errorCode:UNAUTHORIZED Bridge-->>UI: 403 else Key not found Action-->>Bridge: ok:false, errorCode:NOT_FOUND Bridge-->>UI: 404 else Permission denied Action-->>Bridge: ok:false, errorCode:PERMISSION_DENIED Bridge-->>UI: 403 else Last enabled key Action-->>Bridge: ok:false, errorCode:CANNOT_DELETE_LAST_KEY Bridge-->>UI: 400 else Last group key (non-admin) Action-->>Bridge: ok:false, errorCode:CANNOT_DELETE_LAST_GROUP_KEY Bridge-->>UI: 400 else DB error DB-->>Action: throws Error Action-->>Bridge: ok:false, errorCode:DELETE_FAILED Bridge-->>UI: 400 else Success Action->>DB: deleteKey(keyId) DB-->>Action: ok Action-->>Bridge: ok:true Bridge-->>UI: 204 end UI->>UI: errorCode present → getErrorMessage(tErrors, errorCode) else res.errorComments Outside Diff (1)
tests/unit/actions/keys-remove-error-codes.test.ts, line 456-464 (link)await importinside tests shares the cached module instanceEach test does
await import("@/actions/keys")inside the test body. Because Node/Vitest caches modules after the first load, all 7 tests receive the same module object.vi.clearAllMocks()resets mock call counts and queued return values, which is sufficient here, but if a future test needs a fresh module state (e.g. to change a module-level constant) the cached import will silently reuse the old module. The conventional pattern is to hoist the import or usevi.resetModules()/vi.isolateModules()when per-test fresh imports are actually required.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(keys): translate delete errors and c..." | Re-trigger Greptile