feat: add error handling and localization for key loading failures#365
feat: add error handling and localization for key loading failures#365
Conversation
- Added new error message for failed key loading in multiple languages (English, Japanese, Russian, Simplified Chinese, Traditional Chinese). - Enhanced global error component to differentiate between network errors and general errors, providing specific guidance for network issues. - Updated forms to include error handling for provider group loading and key retrieval, improving user feedback on failures.
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's error handling capabilities and user experience by introducing more specific and localized error messages, particularly for key loading failures. It refines the global error display to offer distinct guidance for network connectivity problems versus general errors and integrates robust error management into critical forms. Furthermore, it establishes a standardized pattern for handling server action errors across the application, ensuring consistency and improved feedback. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review
This pull request significantly improves error handling across the application by introducing a global error boundary, a centralized server action hook, and more robust error catching in several components. The changes are well-structured and enhance both user feedback and application stability. My review includes a few suggestions to further improve error handling by providing user-facing feedback in forms where it's currently missing and ensuring all failure paths from server actions are properly handled.
| loadGroups.then(setProviderGroupSuggestions).catch((err) => { | ||
| console.error("[AddKeyForm] Failed to load provider groups:", err); | ||
| }); |
There was a problem hiding this comment.
The error handling for loading provider groups only logs to the console. This provides no feedback to the user, who might be confused about why suggestions aren't appearing. It would be better to show a toast notification to inform the user about the failure. You can use a generic error message from your translation files, for example dashboard.error.loadFailed.
loadGroups.then(setProviderGroupSuggestions).catch((err) => {
console.error("[AddKeyForm] Failed to load provider groups:", err);
toast.error("Failed to load provider groups. Please try again.");
});
| loadGroups.then(setProviderGroupSuggestions).catch((err) => { | ||
| console.error("[EditKeyForm] Failed to load provider groups:", err); | ||
| }); |
There was a problem hiding this comment.
The error handling for loading provider groups only logs to the console. This provides no feedback to the user, who might be confused about why suggestions aren't appearing. It would be better to show a toast notification to inform the user about the failure. You can use a generic error message from your translation files, for example dashboard.error.loadFailed.
loadGroups.then(setProviderGroupSuggestions).catch((err) => {
console.error("[EditKeyForm] Failed to load provider groups:", err);
toast.error("Failed to load provider groups. Please try again.");
});
| .catch((err) => { | ||
| console.error("[UserForm] Failed to load provider groups:", err); | ||
| }); |
There was a problem hiding this comment.
The error handling for loading provider groups only logs to the console. This provides no feedback to the user, who might be confused about why suggestions aren't appearing. It would be better to show a toast notification to inform the user about the failure. You can use a generic error message from your translation files, for example dashboard.error.loadFailed.
.catch((err) => {
console.error("[UserForm] Failed to load provider groups:", err);
toast.error("Failed to load provider groups. Please try again.");
});
| if (keysResult.ok && keysResult.data) { | ||
| setKeys(keysResult.data); | ||
| } |
There was a problem hiding this comment.
The current implementation handles exceptions when calling getKeys, but it doesn't handle the case where the server action returns a failure ({ ok: false, error: '...' }). This can lead to silent failures where the key list fails to load without any user notification.
if (keysResult.ok && keysResult.data) {
setKeys(keysResult.data);
} else if (!keysResult.ok) {
console.error("Failed to load keys:", keysResult.error);
toast.error(t("logs.error.loadKeysFailed"));
}
src/app/[locale]/settings/providers/_components/forms/api-test-button.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Code Review Summary
Overall this is a well-structured PR that adds proper error handling and localization for key loading failures. The changes improve user experience by catching and logging errors that were previously unhandled.
PR Size: M
- Lines changed: 446 (390 additions + 56 deletions)
- Files changed: 14
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 1 | 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 |
Medium Priority Issues (Should Fix)
1. src/app/global-error.tsx:67 - Button container not centered properly
The button container uses display: "flex" with gap: "12px", but the parent container only has textAlign: "center". Text alignment does not center flex children - you need justifyContent: "center" on the flex container to center the buttons horizontally.
Suggested fix:
<div style={{ display: "flex", gap: "12px", justifyContent: "center" }}>Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling - Well done: errors are now logged and user feedback is provided
- Type safety
- Documentation accuracy
- Test coverage - Note: New utility files (
use-server-action.ts,error-detection.ts) are not being used yet in this PR, suggesting they may be infrastructure for future work - Code clarity
Additional Notes
-
The new
useServerActionhook andwithErrorHandlingutility insrc/lib/hooks/use-server-action.tsare well-designed but are not used by any component in this PR. Consider either using them to replace the manual.catch()patterns added in this PR, or moving them to a separate PR. -
The
getSafeErrorMessagefunction inerror-detection.tsalways returns the fallback message and ignores the error parameter (intentionally for security). The function signature is clean but the parameter is unused - this is intentional per the security comment. -
The error handling improvements in the form components (
add-key-form.tsx,edit-key-form.tsx,user-form.tsx, etc.) consistently log errors withconsole.errorand provide user feedback via toast - this is good practice.
Automated review by Claude AI
…-button.tsx Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Summary
Enhances error handling and user experience by improving the global error component to differentiate between network errors and general errors, adding proper error catching in forms and data-loading components, and introducing new utility functions for safe error handling.
Problem
When certain errors occurred (particularly network errors like "Failed to fetch"), the application displayed a full-screen error page with raw error messages, which:
Related Issues:
Solution
Approach
isNetworkError()identifies network-related errors (fetch failures, timeouts, connection refused, etc.)useServerActionhook provides standardized error handling with toast notifications instead of full-screen errorsChanges
Core Changes
src/app/global-error.tsx- Enhanced to detect network errors and show appropriate UI with troubleshooting stepssrc/lib/utils/error-detection.ts- New utility for network error detection and safe error message handlingsrc/lib/hooks/use-server-action.ts- New hook for unified server action execution with error handlingForm Error Handling
src/app/[locale]/dashboard/_components/user/forms/add-key-form.tsx- Added error handling for provider group loadingsrc/app/[locale]/dashboard/_components/user/forms/edit-key-form.tsx- Added error handling for provider group loadingsrc/app/[locale]/dashboard/_components/user/forms/user-form.tsx- Added error handling for provider group loadingsrc/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsx- Added error handling for key loading with toast notificationsrc/app/[locale]/settings/providers/_components/forms/api-test-button.tsx- Added error handling for preset loadingsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx- Added error handling for key retrievalLocalization
loadKeysFailederror message in 5 languages:Breaking Changes
None. All changes are additive and backward-compatible.
Testing
Manual Testing
Security Considerations
Checklist
Description enhanced by Claude AI