fix: fix request not displaying interface error messages correctly #4596
Conversation
|
WalkthroughThe changes in this pull request include updates to error message handling in multiple response functions and the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (16)
apps/backend-mock/utils/response.ts (3)
44-44: Approved: Improved error message readabilityThe change from 'ForbiddenException' to 'Forbidden Exception' improves the readability of the error message. This is a good user experience enhancement.
Consider differentiating between the two arguments passed to
useResponseError. The first argument is typically used as a short error code or type, while the second is a more detailed message. You might want to adjust it like this:return useResponseError('FORBIDDEN', 'Forbidden Exception');This approach maintains the improved readability while providing more structured error information.
49-49: Approved: Consistent improvement in error message readabilityThe change from 'UnauthorizedException' to 'Unauthorized Exception' improves the readability of the error message, consistent with the change made to
forbiddenResponse. This enhances user experience and maintains consistency across error messages.Similar to the suggestion for
forbiddenResponse, consider differentiating between the two arguments passed touseResponseError:return useResponseError('UNAUTHORIZED', 'Unauthorized Exception');This approach provides more structured error information while maintaining the improved readability.
44-49: Overall: Consistent improvement in error messages with room for further enhancementThe changes to both
forbiddenResponseandunAuthorizedResponsefunctions consistently improve the readability of error messages. This aligns well with the PR objective of fixing the display of interface error messages.To further enhance the error handling structure, consider implementing a more comprehensive error system:
Create an enum for error types:
enum ErrorType { FORBIDDEN = 'FORBIDDEN', UNAUTHORIZED = 'UNAUTHORIZED', // Add other error types as needed }Modify the
useResponseErrorfunction to use this enum:export function useResponseError(type: ErrorType, message: string) { return { code: -1, data: null, error: type, message, }; }Update the response functions:
export function forbiddenResponse(event: H3Event<EventHandlerRequest>) { setResponseStatus(event, 403); return useResponseError(ErrorType.FORBIDDEN, 'Forbidden Exception'); } export function unAuthorizedResponse(event: H3Event<EventHandlerRequest>) { setResponseStatus(event, 401); return useResponseError(ErrorType.UNAUTHORIZED, 'Unauthorized Exception'); }This approach would provide a more structured and extensible error handling system, making it easier to manage and expand error types in the future.
apps/web-naive/src/api/request.ts (1)
97-103: Improved error handling, with minor suggestions for enhancement.The changes effectively improve the granularity of error reporting by extracting more specific error messages from the response data. This aligns well with the PR objective of improving error message display.
Consider the following minor improvements:
- Enhance robustness by checking if
error.responseexists:- const responseData = error?.response?.data ?? {}; + const responseData = error?.response?.data ?? {}; + const errorMessage = responseData?.error ?? responseData?.message ?? msg; - const errorMessage = responseData?.error ?? responseData?.message ?? ''; - // 如果没有错误信息，则会根据状态码进行提示 - message.error(errorMessage || msg); + message.error(errorMessage);
- Enhance the comment to provide more context about the error object structure:
- // 当前mock接口返回的错误字段是 error 或者 message + // 错误消息优先级: error.response.data.error > error.response.data.message > msg + // msg 通常包含基于 HTTP 状态码的通用错误消息These changes will make the error handling more robust and the code more self-documenting.
apps/web-antd/src/api/request.ts (1)
98-104: Improved error handling, but further enhancements possibleThe changes improve the error handling by extracting more specific error messages from the response data. This aligns well with the PR objective of fixing the display of interface error messages. However, there are a couple of suggestions for further improvement:
Update the comment on line 99 to reflect the new implementation. The current comment is outdated.
Add a null check for
error.responsebefore accessing its properties to make the error handling more robust.Consider applying the following changes:
client.addResponseInterceptor( errorMessageResponseInterceptor((msg: string, error) => { - // 这里可以根据业务进行定制,你可以拿到 error 内的信息进行定制化处理，根据不同的 code 做不同的提示，而不是直接使用 message.error 提示 msg - // 当前mock接口返回的错误字段是 error 或者 message - const responseData = error?.response?.data ?? {}; + // Extract specific error messages from the response data, falling back to the original msg if not found + const responseData = error?.response?.data ?? {}; const errorMessage = responseData?.error ?? responseData?.message ?? ''; - // 如果没有错误信息，则会根据状态码进行提示 message.error(errorMessage || msg); }), );This change updates the comment to accurately describe the new implementation and removes the unnecessary comment about mock interfaces.
playground/src/api/request.ts (1)
99-105: Improved error handling, but consider further enhancements.The changes to the error handling logic are good improvements. The code now extracts more detailed error messages from the response, handling different error response structures.
However, consider the following suggestions for further improvement:
- Implement more specific error handling based on error codes or types.
- Add error logging for debugging purposes.
- Consider using a custom error class to standardize error handling across the application.
Here's a suggested refactor to address these points:
import { logger } from '@/utils/logger'; // Assume a logger utility exists // Custom error class class ApiError extends Error { constructor(public code: number, message: string) { super(message); this.name = 'ApiError'; } } // ... client.addResponseInterceptor( errorMessageResponseInterceptor((msg: string, error) => { const responseData = error?.response?.data ?? {}; const errorCode = responseData?.code ?? error?.response?.status ?? 500; const errorMessage = responseData?.error ?? responseData?.message ?? msg; // Log the error logger.error(`API Error ${errorCode}: ${errorMessage}`, { error }); // Create a custom error const apiError = new ApiError(errorCode, errorMessage); // Handle specific error codes switch (errorCode) { case 400: message.error('Bad Request: ' + errorMessage); break; case 401: message.error('Unauthorized: ' + errorMessage); // Potentially trigger a re-authentication flow break; case 404: message.error('Not Found: ' + errorMessage); break; default: message.error(errorMessage || msg); } throw apiError; }), );This refactored version includes error logging, a custom
ApiErrorclass, and more specific error handling based on error codes.apps/web-ele/src/api/request.ts (1)
98-104: Improved error handling, but consider further enhancementsThe changes to the error handling logic are good improvements:
- Renaming
_errortoerrorimproves code clarity.- The new error message extraction logic provides more robust handling of different API response structures.
However, consider the following suggestions for further improvement:
- Extract the error message logic into a separate function for better readability and testability:
function extractErrorMessage(error: any, defaultMsg: string): string { const responseData = error?.response?.data ?? {}; return responseData?.error ?? responseData?.message ?? defaultMsg; }
Consider adding more granular error handling based on HTTP status codes or specific error codes from your API. This could help provide more specific error messages to the user.
Add a type for the
errorparameter to improve type safety:errorMessageResponseInterceptor((msg: string, error: AxiosError) => { // ... (rest of the code) })
- Consider using a logging service for errors, which can help with debugging and monitoring in production.
Would you like me to provide a more detailed implementation of these suggestions?
docs/src/guide/essentials/server.md (4)
Line range hint
1-89: LGTM! Clear introduction and configuration instructions.The introduction and local development configuration sections are well-structured and informative. The use of code blocks for configuration examples is helpful.
Consider adding a brief explanation of why CORS is necessary for local development, as this might not be immediately clear to junior developers.
Line range hint
90-110: Good coverage of production environment setup.The section on production environment configuration provides essential information, including the useful tip about dynamically modifying the API address.
Consider expanding the cross-origin handling section for production. A brief example of Nginx configuration for API proxying could be beneficial for developers new to deployment.
Line range hint
111-270: Comprehensive API request configuration and examples.The API request configuration section is well-documented with clear examples for different HTTP methods. The request client configuration is thorough, covering important aspects like authentication, token refresh, and error handling.
For consistency, consider using the same function name
saveUserApifor all examples in the POST/PUT request section, but with different comments to distinguish between POST and PUT operations.
Line range hint
305-341: Informative data mocking section with clear instructions.The data mocking section effectively explains the use of Nitro for local mock data handling and provides clear instructions on how to disable mock services if needed. The note about production mocking being unsupported is an important clarification.
Consider adding a brief explanation or link to resources on best practices for transitioning from mock data to real API endpoints when moving to production. This could help developers plan for the transition more effectively.
docs/src/en/guide/essentials/server.md (5)
258-264: Improved error handling logic. Consider adding type safety.The updated error handling provides more granular control and better error message extraction. This is a good improvement that allows for more flexible error reporting based on the API response structure.
Consider adding type safety to the
errorobject to avoid potential runtime errors:errorMessageResponseInterceptor((msg: string, error: AxiosError) => { const responseData = (error?.response?.data as Record<string, unknown>) ?? {}; const errorMessage = (responseData?.error ?? responseData?.message ?? '') as string; message.error(errorMessage || msg); }),This change ensures type safety and makes the code more robust against unexpected response structures.
Line range hint
291-321: Clear instructions for implementing refresh token functionality. Consider adding error handling.The new section on Refresh Token provides clear and concise instructions for enabling and implementing the refresh token feature. The step-by-step approach and code examples are helpful for developers.
Consider adding a note about error handling in the
doRefreshTokenfunction:async function doRefreshToken() { try { const accessStore = useAccessStore(); // Adjust this to your refresh token API const resp = await refreshTokenApi(); const newToken = resp.data; accessStore.setAccessToken(newToken); return newToken; } catch (error) { console.error('Failed to refresh token:', error); // Consider implementing a fallback strategy or user notification here throw error; } }This addition would make the example more robust and remind developers to handle potential errors in the token refresh process.
Line range hint
323-341: Improved mocking strategy with clear instructions. Consider adding a note on transitioning from mocks to real APIs.The updates to the Data Mocking section, including the removal of production environment mocking and the introduction of Nitro for local mocking, are significant improvements. These changes enhance security and provide a more realistic development environment.
Consider adding a brief note on best practices for transitioning from mock data to real APIs:
### Transitioning from Mock to Real APIs When moving from development to production, ensure a smooth transition from mock data to real APIs: 1. Gradually replace mock endpoints with real API calls in your code. 2. Use feature flags or environment variables to switch between mock and real APIs during the transition phase. 3. Thoroughly test the application with real APIs in a staging environment before deploying to production. 4. Update documentation and team communication to reflect the shift from mock data to real APIs.This addition would provide valuable guidance for developers on managing the transition from development to production environments.
Line range hint
343-349: Clear instructions for disabling mock service. Consider adding a note on implications.The new section on Disabling Mock Service provides clear and concise instructions for turning off the mock service when it's not needed. This is a useful addition for developers who may not require mocking or are ready to switch to real APIs.
Consider adding a note about the implications of disabling the mock service:
Note: When disabling the mock service, ensure that: 1. All API endpoints in your application are updated to point to real backend services. 2. The real backend services are available and accessible from your development environment. 3. Any environment-specific configurations (like API URLs) are properly set up. 4. The team is informed about the change to prevent confusion during development.This addition would help developers understand the full impact of disabling the mock service and prepare accordingly.
Line range hint
1-349: Comprehensive update improves documentation quality. Consider adding a table of contents.The extensive restructuring and content updates have significantly improved the quality and comprehensiveness of this documentation. The document now provides clear guidance on server interaction, API configuration, and data mocking for both development and production environments.
To further enhance the usability of this document, consider adding a table of contents at the beginning:
## Table of Contents 1. [Server Interaction and Data Mocking](#server-interaction-and-data-mocking) 2. [Interaction in Development Environment](#interaction-in-development-environment) 2.1 [Local Development CORS Configuration](#local-development-cors-configuration) 2.2 [API Requests](#api-requests) 3. [Production Environment Interaction](#production-environment-interaction) 4. [API Request Configuration](#api-request-configuration) 5. [Refresh Token](#refresh-token) 6. [Data Mocking](#data-mocking) 7. [Disabling Mock Service](#disabling-mock-service)This addition would improve navigation, especially for longer documents, allowing readers to quickly jump to relevant sections.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- apps/backend-mock/utils/response.ts (1 hunks)
- apps/web-antd/src/api/request.ts (1 hunks)
- apps/web-ele/src/api/request.ts (1 hunks)
- apps/web-naive/src/api/request.ts (1 hunks)
- docs/src/en/guide/essentials/server.md (1 hunks)
- docs/src/guide/essentials/server.md (1 hunks)
- packages/effects/request/src/request-client/preset-interceptors.ts (1 hunks)
- playground/src/api/request.ts (1 hunks)
- playground/src/views/examples/vxe-table/remote.vue (0 hunks)
💤 Files with no reviewable changes (1)
- playground/src/views/examples/vxe-table/remote.vue
🧰 Additional context used
🔇 Additional comments (3)
apps/web-antd/src/api/request.ts (1)
Line range hint
1-104: Overall improvement in error handlingThe changes in this file significantly improve the error handling mechanism, particularly in the
errorMessageResponseInterceptor. The modifications align well with the PR objective of fixing the display of interface error messages. The new implementation provides more granular and informative error messages to the user by extracting specific error information from the response data.While the changes are positive, consider implementing the suggested refinements to further enhance the code quality and robustness.
packages/effects/request/src/request-client/preset-interceptors.ts (1)
94-94: Clarify the rationale for simplifying error message initializationThe change from a complex initialization (likely using response data) to a simple empty string initialization may impact error message specificity. While this simplification ensures consistent error messages based on HTTP status codes, it might lead to the loss of detailed error information previously available in the response data.
Could you please clarify the reasoning behind this change? Was there an issue with the previous implementation that this addresses?
Consider a hybrid approach that maintains both specific and general error messages:
let errorMessage = error?.response?.data?.error?.message || ''; // ... rest of the function if (!errorMessage) { switch (status) { // ... existing switch cases } }This approach would preserve any specific error messages from the server while falling back to general messages based on status codes when needed.
docs/src/guide/essentials/server.md (1)
Line range hint
271-304: Clear instructions for token refresh implementation.The token refresh section effectively explains how to enable and configure token refresh functionality. The provided code snippets for token formatting and refresh logic are clear and helpful.
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style
releaseDatecolumn formatting.