-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
fix: fix the issues with the request.js (#4667) #4669
Conversation
|
WalkthroughThe changes in this pull request focus on modifying the error handling logic within the response interceptor of the Changes
Possibly related PRs
Suggested labels
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 (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- apps/web-antd/src/api/request.ts (1 hunks)
- playground/src/api/request.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
playground/src/api/request.ts (1)
82-82
:⚠️ Potential issueConsider the implications of changing the error structure
The modification to throw the entire response object instead of a constructed error message offers more flexibility in error handling. However, this is a breaking change that requires careful consideration:
- This change will affect all downstream error handling code that expects the error to be a string.
- The new error structure provides more information but may lead to type errors if not properly handled.
To address these concerns:
- Document the new error structure in the codebase, explaining what information is available and how to access it.
- Update all error handling code throughout the application to accommodate the new error structure.
- Consider adding a type definition for the new error structure to ensure type safety:
type RequestError = { response: { data: any; status: number; statusText: string; headers: any; config: any; }; };
- Implement a migration strategy to gradually update error handling across the application, possibly by providing a compatibility layer during the transition.
To assess the impact of this change, run the following script to find potential affected areas:
This will help identify areas of the codebase that may need updating due to the new error structure.
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.
采用 @anncwb 的建议,throw 对象以用于后面处理错误拿到的数据结构有效,401判断能命中
pr没校验通过。这个分支改善了,https://github.com/vbenjs/vue-vben-admin/pull/4679。 |
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
Bug Fixes
Chores