-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
refactor: error handling logic #56
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduced in this pull request involve the implementation of a new error handling mechanism across three adapter files: Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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 (
|
✅ Deploy Preview for uni-network ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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
🧹 Outside diff range and nitpick comments (1)
packages/core/src/adapters/download.ts (1)
Line range hint
1-67
: Consider extracting common error handling logicAll three adapters (request, upload, download) share very similar error handling patterns. Consider extracting this common logic into a shared utility function to reduce code duplication and ensure consistent error handling across adapters.
Example implementation:
// utils/errorHandler.ts export const createErrorHandler = <T extends UniApp.RequestTask | UniApp.UploadTask | UniApp.DownloadTask>( config: UnConfig, task: T | undefined ) => (err: UniApp.GeneralCallbackResult) => { return new UnError(err.errMsg, UnError.ERR_NETWORK, config, task); }; // Usage in adapters fail: createErrorHandler(config, task),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/core/src/adapters/download.ts
(2 hunks)packages/core/src/adapters/request.ts
(2 hunks)packages/core/src/adapters/upload.ts
(2 hunks)
🔇 Additional comments (6)
packages/core/src/adapters/request.ts (2)
4-4
: LGTM: Clean import addition
The UnError import is properly placed with other error-related imports, maintaining good code organization.
56-57
: LGTM: Improved error handling implementation
The new error handling approach using UnError provides better error context and aligns with the PR's objective of standardizing error handling. The implementation correctly includes:
- Error message from the original error
- Network error code classification
- Complete request configuration
- Task reference for additional context
packages/core/src/adapters/upload.ts (2)
4-4
: LGTM: Consistent import structure
The UnError import maintains consistency with other adapter files.
59-59
: LGTM: Unified error handling for uploads
The error handling implementation maintains consistency with the request adapter while properly handling upload-specific task types.
packages/core/src/adapters/download.ts (2)
4-4
: LGTM: Consistent import structure
The UnError import maintains consistency with other adapter files.
67-67
: LGTM: Unified error handling for downloads
The error handling implementation maintains consistency with other adapters while properly handling download-specific task types.
- Introduce `UnError` class to unify error handling - Replace existing error handling methods with `UnError` to encapsulate error information - Optimize code structure to improve consistency and readability of error handling
UnError
class to unify error handlingUnError
to encapsulate error information重构错误处理逻辑
Description 描述
经测试,uni.request、uni.uploadFile、uni.download等方法,即使服务器返回4xx、5xx等错误状态码,也只会触发success回调,只有客户端网络不通或者服务器证书配置错误等服务器完全无法返回响应的情况,才会触发fail回调。参考请求库axios的逻辑以上情况应该进入全局响应拦截器的失败回调而不是全局响应拦截器的成功回调。
此外,本PR也将修复一个存在很久的BUG:因网络问题导致的请求失败情况,在全局响应拦截器中的回调函数无法获得请求配置的问题。
Linked Issues 关联的 Issues
#49
Additional context 额外上下文
借鉴axios相关处理:
https://github.com/axios/axios/blob/v0.x/lib/adapters/xhr.js
https://github.com/axios/axios/blob/v1.x/lib/adapters/xhr.js
Summary by CodeRabbit
New Features
UnError
class, enhancing error reporting consistency.Bug Fixes