-
Notifications
You must be signed in to change notification settings - Fork 898
feat: better error messages #439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes improve error handling in the API key management functionality by categorizing errors based on HTTP status codes in the Changes
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/api/ApiKey.tsx (1)
48-72
: Good error handling implementation, with some suggestions for improvement.The error handling is well-structured and provides specific feedback based on different error scenarios. Here are some suggestions to enhance it further:
+const ERROR_KEYS = { + 401: 'unauthorized', + 404: 'not_found', + 429: 'rate_limit', + 500: 'server', +} as const; + const fetchApiKey = async () => { try { const { data } = await axios.get(`${apiUrl}/auth/api-key`); setApiKey(data.api_key); notify('success', t('apikey.notifications.success.fetch')); } catch (error: any) { const status = error.response?.status; - let errorKey = 'unknown'; - - switch (status) { - case 401: - errorKey = 'unauthorized'; - break; - case 404: - errorKey = 'not_found'; - break; - case 500: - errorKey = 'server'; - break; - default: - if (error.message?.includes('Network Error')) { - errorKey = 'network'; - } - } + const errorKey = ERROR_KEYS[status] || + (axios.isAxiosError(error) && !error.response ? 'network' : 'unknown'); notify( 'error', t(`apikey.notifications.errors.fetch.${errorKey}`, { error: error.response?.data?.message || error.message }) );Changes suggested:
- Define error keys as constants for better maintainability
- Add rate limit (429) handling
- Use object literal instead of switch statement
- Use
axios.isAxiosError
for more robust network error detection
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/api/ApiKey.tsx
(1 hunks)
🔇 Additional comments (1)
src/components/api/ApiKey.tsx (1)
46-46
: LGTM!Good addition of success notification to provide immediate feedback to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
server/src/routes/auth.ts (2)
284-288
: Consider adding a success code for consistency.While the success response includes the
ok
field, consider adding acode
field for consistency with error responses.return res.status(200).json({ ok: true, + code: "success", message: "API key fetched successfully", api_key: user.api_key || null, });
264-295
: Consider standardizing error responses across all routes.The improved error handling structure in the
/api-key
endpoint provides better clarity. Consider applying this pattern to other routes in the file for consistency.The standardized response structure includes:
ok
boolean fielderror
message fieldcode
for error categorizationExample routes that could benefit from this pattern:
/register
/login
/logout
/current-user
/user/:id
/generate-api-key
/delete-api-key
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/src/routes/auth.ts
(2 hunks)
🔇 Additional comments (3)
server/src/routes/auth.ts (3)
264-268
: LGTM! Improved unauthorized error response.The standardized response structure with
ok
andcode
fields enhances error handling clarity.
277-281
: LGTM! Enhanced user not found error response.The standardized response structure with
ok
andcode
fields improves error handling clarity.
290-295
: LGTM! Improved server error response and logging.The addition of error logging and standardized error response structure enhances debugging capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
public/locales/zh.json (1)
143-170
: Enhanced Nested Error Structure for API Key Management
The restructured nested error messages (under keys such as "fetch", "generate", "delete", and "copy") now provide a much more granular level of detail for failures (network issues, unauthorized access, not found, server errors, and unknown errors). Please double-check that all function calls in the front end reference these new keys correctly.public/locales/ja.json (1)
143-170
: Improved Error Message Granularity in Japanese Localization
The introduction of a nested error structure that encapsulates distinct error messages for "fetch", "generate", "delete", and "copy" operations improves precision. Please confirm that the error keys match the ones sent by the backend and are handled correctly in the UI.public/locales/es.json (1)
143-170
: Mejora en la estructura anidada de los mensajes de error
La nueva organización de los mensajes de error en categorías ("fetch", "generate", "delete" y "copy") mejora notablemente la claridad del feedback en caso de fallo. Verifique que esta estructura se refleje de forma coherente en los mensajes mostrados al usuario.public/locales/de.json (1)
142-168
: Verbesserte Fehlerstruktur in der deutschen Lokalisierung
Die Umstrukturierung der Fehlermeldungen in verschachtelte Kategorien (unter "fetch", "generate", "delete" und "copy") erleichtert das differenzierte Fehlermanagement. Bitte überprüfen Sie, dass alle API-Antworten diese neuen Schlüssel unterstützen und die front-end Logik entsprechend angepasst wurde.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locales/de.json
(1 hunks)public/locales/en.json
(1 hunks)public/locales/es.json
(1 hunks)public/locales/ja.json
(1 hunks)public/locales/zh.json
(1 hunks)server/src/routes/auth.ts
(4 hunks)src/components/api/ApiKey.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/src/routes/auth.ts
- src/components/api/ApiKey.tsx
🔇 Additional comments (6)
public/locales/zh.json (1)
172-177
: Structured Success Notifications for API Key Operations
The newly added “success” messages clearly distinguish the outcomes for fetching, generating, deleting, and copying API keys. This will help deliver clearer user feedback. No issues noted.public/locales/ja.json (1)
172-177
: Clear Success Feedback
The clear, segmented success messages now distinctly indicate when an API key is successfully retrieved, generated, deleted, or copied. This consistency will lead to improved user feedback across the application.public/locales/en.json (2)
143-170
: Detailed Error Messages Enhancement
The detailed error messages (covering network, unauthorized, not found, server, and unknown errors) now provide extra diagnostic information. This structure is beneficial for both debugging and user support. Please verify that all API endpoints now return error codes matching these keys.
172-177
: Success Message Structuring
The success messages for each action are now differentiated and clear, providing explicit end-user feedback after operations. Ensure that any client-side mapping of these responses is updated accordingly.public/locales/es.json (1)
172-177
: Mensajes de éxito bien segmentados
El agregado de mensajes de éxito específicos para cada operación de la clave API ofrece una retroalimentación precisa para el usuario. Todo luce correcto; asegúrese de que la lógica de presentación utilice estos nuevos mensajes.public/locales/de.json (1)
171-176
: Klare Erfolgsmeldungen für API-Schlüssel-Operationen
Die neuen Erfolgsmeldungen liefern präzise Rückmeldungen (für das Abrufen, Generieren, Löschen und Kopieren von API-Schlüsseln), was die Benutzerfreundlichkeit erhöht.
@amhsirak Review |
Summary by CodeRabbit
Bug Fixes
New Features