-
Notifications
You must be signed in to change notification settings - Fork 10
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(): try to reload when specified error occurs #4588
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/brick-container/src/bootstrap.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@next-core/eslint-config-next" to extend from. Please check that the name of the config is correct. The config "@next-core/eslint-config-next" was referenced from the config file in "/.eslintrc". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 总览这个拉取请求引入了一系列与运行时错误处理和网络错误相关的改进。主要变更包括在运行时模块中添加新的错误检测和重载函数,更新引导程序的错误处理逻辑,并在国际化文件中添加网络错误消息。这些修改旨在增强应用程序在遇到网络错误时的鲁棒性和用户体验。 变更
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
🧹 Nitpick comments (1)
packages/runtime/src/shouldReloadForError.ts (1)
6-16
: 谨慎使用sessionStorage
计数机制
在网络错误时根据计数重载页面,逻辑较清晰。但请确保在其他场景下不会误写或误删sessionStorage
,以免影响用户体验。如果未来有更多场景需要依赖该计数,建议封装工具函数集中管理。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (10)
etc/runtime.api.md
(3 hunks)packages/brick-container/package.json
(2 hunks)packages/brick-container/src/bootstrap.ts
(2 hunks)packages/brick-container/src/i18n.ts
(2 hunks)packages/runtime/src/index.ts
(1 hunks)packages/runtime/src/internal/Router.ts
(3 hunks)packages/runtime/src/isNetworkError.spec.ts
(1 hunks)packages/runtime/src/isNetworkError.ts
(1 hunks)packages/runtime/src/shouldReloadForError.spec.ts
(1 hunks)packages/runtime/src/shouldReloadForError.ts
(1 hunks)
🔇 Additional comments (34)
packages/runtime/src/isNetworkError.ts (2)
1-3
: 导入依赖看起来合理
导入HttpFetchError
与BrickLoadError
能够准确区分网络请求异常与模块加载异常,对后续逻辑十分有帮助。
4-14
: 逻辑封装清晰,便于统一判断网络错误
该函数综合判断多种类型的错误(包括不同自定义错误和原生事件类型),能让上层调用对网络错误的识别更加一致,值得肯定。packages/runtime/src/shouldReloadForError.ts (1)
18-20
: 重置功能简洁易懂
resetReloadForError
函数对清除计数非常直接,如有更多逻辑需求可在此处拓展。packages/brick-container/src/i18n.ts (3)
5-5
: 新增枚举值命名明晰
新枚举值NETWORK_ERROR
命名直观,能在错误处理逻辑中快速定位到网络异常场景。
15-15
: 英文翻译准确
"Network Error"
可以帮助用户清楚辨识网络相关的问题,与其他错误文案风格保持一致。
23-23
: 中文翻译准确
"网络错误"
与英文翻译含义一致,方便国内用户理解。packages/runtime/src/index.ts (1)
44-45
: 导出新模块增强可复用性
将isNetworkError
与shouldReloadForError
统一通过主入口导出,为其他模块或外部系统使用提供了更加便捷的路径。packages/runtime/src/isNetworkError.spec.ts (9)
1-2
: 导入依赖的范围符合测试需求
这里引入了HttpFetchError
与BrickLoadError
,可保证对网络错误类型进行充分覆盖。
3-4
: 确保路径引用的正确性
引用的isNetworkError.js
测试目标文件路径正确,应与项目结构保持一致。
5-9
: 测试用例命名规范和可读性良好
“should return true for BrickLoadError” 准确描述预期行为,测试断言明确。
11-14
: 测试覆盖 HttpFetchError 场景
HttpFetchError
的断言与实际逻辑相符,达成网络错误判断的完整性。
16-20
: ChunkLoadError 测试场景完整
通过修改error.name
模拟特定错误名称,可验证此类异常的网络错误判定正确性。
22-27
: DOM Event 对象的网络错误识别测试
对Event
类型的判断充分体现了边界检测,且伪造target
逻辑合理。
29-32
: 对其他普通错误的排除逻辑
此处确保非网络错误返回false
,保证判断函数的负向覆盖场景。
34-37
: 对 null 和 undefined 入参的安全校验
验证空值输入可避免意外报错,测试场景覆盖充分。
39-42
: 非 Error 实例输入的断言
此处可防止误判任意对象为网络错误,增强兼容性与鲁棒性。packages/runtime/src/shouldReloadForError.spec.ts (7)
1-2
: 导入功能函数的可测性良好
成功导入shouldReloadForError
及依赖BrickLoadError
。
4-6
: 本地存储的相关方法已被正确监听
利用jest.spyOn
使得读写操作可追踪,测试精确度提升。
8-16
: 测试环境初始化配置完善
在beforeEach
中设置userAgent
与模拟getItem
返回值,有助于测试独立与干净。
17-22
: 测试网络错误且计数未达上限的重载逻辑
断言返回true
并调用setItem
,符合预期,确保了计数的正确累加。
24-32
: 测试计数达上限的场景
通过指向模拟值"2"
,验证计数已达上限且返回false
,并断言移除计数项,逻辑清晰。
34-44
: 测试与特定 userAgent 匹配的依赖关系
当userAgent
不符合条件时,函数正确返回false
,且不更新计数。
46-51
: 区分非网络错误的场景
确认返回false
并且未更新计数,保证对常规错误不触发重载。packages/brick-container/src/bootstrap.ts (5)
6-7
: 新增引入重载逻辑的函数
在此处引入shouldReloadForError
与resetReloadForError
,为后续错误处理提供基础。
9-9
: 同步引入isNetworkError
增强基于错误类型的判定,在后续可针对网络错误赋予更精确的文案。
141-141
: 重置重载计数
调用resetReloadForError
后,可避免成功初始化后仍遗留上次错误的计数。
147-151
: 实现特定错误触发页面刷新
若shouldReloadForError
判断为需刷新时,直接调用location.reload()
并返回failed
,逻辑简明。
158-160
: 细化网络错误提示文案
通过isNetworkError
判断使用NETWORK_ERROR
文案,加深用户对错误类型的理解。packages/runtime/src/internal/Router.ts (3)
59-62
: 引入重载功能函数
新增对resetReloadForError
与shouldReloadForError
的导入,提升路由错误的可控性。
433-436
: 在路由调用栈中判定特定错误后触发刷新
当满足shouldReloadForError
的条件则执行刷新,保证对网络级别重载需求的即时响应。
517-517
: 重置错误计数的最佳时机
在渲染流程结束时统一调用resetReloadForError
,有效防止意外循环重载。packages/brick-container/package.json (2)
45-45
: 依赖版本更新看起来合理http-proxy-middleware 的小版本更新(3.0.2 -> 3.0.3)应该包含了一些bug修复。
65-65
: 新增依赖关系配置正确@next-core/utils 作为新依赖同时被添加到 devDependencies 和 peerDependencies 中,这符合微服务架构中的最佳实践。
Also applies to: 71-71
etc/runtime.api.md (1)
253-255
: 新增网络错误检测函数isNetworkError 函数的添加有助于统一处理网络相关错误,这是一个很好的抽象。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v3 #4588 +/- ##
=======================================
Coverage 95.20% 95.21%
=======================================
Files 206 208 +2
Lines 8940 8958 +18
Branches 1707 1711 +4
=======================================
+ Hits 8511 8529 +18
Misses 321 321
Partials 108 108
|
next-core Run #10858
Run Properties:
|
Project |
next-core
|
Branch Review |
steve/v3-reload-for-error
|
Run status |
Passed #10858
|
Run duration | 00m 22s |
Commit |
7d7f43a108 ℹ️: Merge c91b3d264f1e8ed09e575864ea7aefacb71307c9 into a66abb00c5ef6e134f4b864bee87...
|
Committer | Shenwei Wang |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
16
|
View all changes introduced in this branch ↗︎ |
c8e1679
to
c91b3d2
Compare
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
🧹 Nitpick comments (2)
etc/runtime.api.md (1)
528-530
: 建议完善错误重载机制的文档shouldReloadForError 和 resetReloadForError 函数组合使用,为错误处理提供了灵活的重载机制。建议:
- 在文档中添加使用示例
- 说明重载的触发条件和限制
packages/runtime/src/internal/Runtime.spec.ts (1)
668-676
: 为myTimeoutProvider
新增字符串类型fail
参数。
将fail
扩展为字符串能更灵活地模拟不同错误场景。可考虑在文档或注释中补充说明参数含义,以便他人快速理解。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (11)
etc/runtime.api.md
(3 hunks)packages/brick-container/package.json
(2 hunks)packages/brick-container/src/bootstrap.ts
(2 hunks)packages/brick-container/src/i18n.ts
(2 hunks)packages/runtime/src/index.ts
(1 hunks)packages/runtime/src/internal/Router.ts
(3 hunks)packages/runtime/src/internal/Runtime.spec.ts
(7 hunks)packages/runtime/src/isNetworkError.spec.ts
(1 hunks)packages/runtime/src/isNetworkError.ts
(1 hunks)packages/runtime/src/shouldReloadForError.spec.ts
(1 hunks)packages/runtime/src/shouldReloadForError.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/runtime/src/index.ts
- packages/brick-container/src/i18n.ts
- packages/runtime/src/shouldReloadForError.ts
- packages/runtime/src/isNetworkError.ts
- packages/runtime/src/internal/Router.ts
- packages/brick-container/package.json
- packages/runtime/src/shouldReloadForError.spec.ts
- packages/runtime/src/isNetworkError.spec.ts
- packages/brick-container/src/bootstrap.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/runtime/src/internal/Runtime.spec.ts
[error] 1744-1745: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (7)
etc/runtime.api.md (2)
253-255
: 函数签名设计合理函数使用
unknown
类型作为参数,符合 TypeScript 的类型安全最佳实践。
401-403
: 需要补充 resetReloadForError 的测试用例根据之前的评审记录,resetReloadForError 函数缺少测试覆盖。建议添加单元测试以确保其功能的正确性和稳定性。
packages/runtime/src/internal/Runtime.spec.ts (5)
4-4
: 引入新错误类型以增强错误处理逻辑。
该导入在本文件启用对BrickLoadError
的测试,可读性和可维护性都很清晰。
19-19
: 引入shouldReloadForError
模块。
这个导入用于决定在出现特定错误时是否执行页面重载,逻辑上与后续测试用例相吻合。
26-26
: 对shouldReloadForError
进行 jest.mock 的注意事项。
此处的 mock 能模拟不同错误触发页面重载的行为,但需谨慎以防止测试结果被错误的模拟逻辑干扰。
585-602
: 为brick-load-error
路由新增上下文配置。
这里在运行时环境中为“砖块加载失败”场景引入了新的路由配置,并模拟了fail === "BrickLoadError"
时抛出的异常。测试覆盖面显著增强。
1741-1775
: 测试“在出现砖块加载错误后自动重载”的场景。
此处测试了若shouldReloadForError
返回真值,页面将通过window.location.reload
强制刷新。该逻辑与设计目标一致,但请保持对错误类型(如BrickLoadError
)的清楚区分并在后续维护中小心处理跳转、缓存等问题。🧰 Tools
🪛 Biome (1.9.4)
[error] 1744-1745: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
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.
Copilot reviewed 6 out of 12 changed files in this pull request and generated no comments.
Files not reviewed (6)
- packages/brick-container/package.json: Language not supported
- etc/runtime.api.md: Evaluated as low risk
- packages/brick-container/src/bootstrap.ts: Evaluated as low risk
- packages/brick-container/src/i18n.ts: Evaluated as low risk
- packages/runtime/src/index.ts: Evaluated as low risk
- packages/runtime/src/internal/Router.ts: Evaluated as low risk
const MAX_RELOAD_COUNT = 2; | ||
|
||
export function shouldReloadForError(error: unknown): boolean { | ||
if (/upchat/i.test(navigator.userAgent) && isNetworkError(error)) { |
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.
仅该标识的客户端网络有异常,针对性处理
INFRA-0
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat
作为提交类型。BREAKING CHANGE: 你的变更说明
。新特性:
feat
作为提交类型。问题修复:
fix
作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore
,docs
,test
等作为提交类型。Summary by CodeRabbit
新功能
Bug 修复
本地化
依赖更新
http-proxy-middleware
到^3.0.3
@next-core/utils
依赖国际化