-
Notifications
You must be signed in to change notification settings - Fork 11
fix(): emit error callback for message.subscribe when connect failed … #4566
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
Walkthrough此次更改涉及多个文件,主要集中在 Changes
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/easyops-runtime/src/websocket/MessageDispatcher.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 (
|
…after retry limit exceeded
1350bab to
1a6efa6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #4566 +/- ##
==========================================
- Coverage 95.23% 95.22% -0.01%
==========================================
Files 206 206
Lines 8917 8924 +7
Branches 1700 1700
==========================================
+ Hits 8492 8498 +6
Misses 319 319
- Partials 106 107 +1
|
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-emit-on-message-close
|
| Run status |
|
| Run duration | 00m 23s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
16
|
| View all changes introduced in this branch ↗︎ | |
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 (2)
packages/easyops-runtime/src/websocket/MessageDispatcher.ts (1)
129-147: 消息处理逻辑的重构改进了代码结构通过分离 message 和 close 事件的处理函数,代码的可维护性得到了提升。建议为复杂的条件判断添加更详细的注释。
建议添加如下注释来解释条件判断的逻辑:
if ( (isSuccess || isFailed) && - // Put this after event type checks, to prevent unnecessary - // JSON stringify. + // 1. 检查事件类型(成功或失败) + // 2. 将身份验证放在事件类型检查之后,以避免不必要的 JSON 序列化 + // 3. 确保消息属于当前的订阅/取消订阅操作 identity === getIdentity(response.payload) ) {etc/runtime.api.md (1)
477-478: 消息分发器的事件监听器现在支持取消订阅功能
onMessage和onClose方法现在返回取消订阅函数,这是一个很好的改进,可以帮助防止内存泄漏并支持在连接失败时进行适当的清理。建议在使用这些方法时遵循以下最佳实践:
- 始终在组件卸载时调用返回的取消订阅函数
- 在重试失败后使用取消订阅函数来清理事件监听器
- 考虑使用 React 的 useEffect 钩子(如果适用)来自动处理订阅的生命周期
示例用法:
const unsubscribe = messageDispatcher.onMessage("channel", (data) => { // 处理消息 }); // 在组件卸载时清理 useEffect(() => { return () => { unsubscribe(); }; }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
etc/runtime.api.md(1 hunks)packages/easyops-runtime/src/websocket/MessageDispatcher.ts(3 hunks)packages/easyops-runtime/src/websocket/MessageService.ts(2 hunks)packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts(2 hunks)packages/runtime/src/internal/Runtime.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/easyops-runtime/src/websocket/MessageService.ts
- packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts
- packages/runtime/src/internal/Runtime.ts
🔇 Additional comments (3)
packages/easyops-runtime/src/websocket/MessageDispatcher.ts (3)
7-7: 导入 RETRY_LIMIT 常量的修改是合理的
从 MessageService.js 导入 RETRY_LIMIT 常量有助于在整个模块中保持重试限制的一致性。
102-103: onClose 方法返回值的修改提升了事件监听器的管理能力
现在的实现允许调用者获取取消订阅的函数,这样可以在适当的时候清理事件监听器,防止内存泄漏。
151-159: 错误处理的改进增强了与 V2 版本的兼容性
通过添加 retryCount 属性,确保了在重试限制超出时能够提供与 V2 版本兼容的错误信息。建议验证这个改动在不同版本中的行为是否一致。
运行以下脚本来验证相关代码:
✅ Verification successful
Based on the search results, I can now verify the compatibility concerns. The code shows clear evidence of maintaining compatibility between V2 and V3 versions:
- In MessageService.ts, we see the RETRY_LIMIT constant defined and the retry mechanism implementation
- The code explicitly mentions "Keep behavior as v2: emit close event only after retry limit exceeded"
- The retryCount handling is consistent across the implementation
Let me generate the final response:
WebSocket 错误处理的兼容性已得到验证
代码实现确实保持了与 V2 版本的向后兼容性:
- 在连接重试超出限制时,会发送包含 retryCount 的错误信息
- 保持了 V2 版本的行为:仅在重试次数超出限制后才触发关闭事件
- 重试机制的实现完整且一致
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查 V2 和 V3 版本中的重试处理逻辑差异
# 搜索与重试相关的代码
rg -A 5 "retryCount|RETRY_LIMIT" --type ts
# 搜索错误处理相关的代码
ast-grep --pattern 'reject($_)'
Length of output: 6674
…after retry limit exceeded
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
RETRY_LIMIT常量,以改进错误处理逻辑。改进
onClose和onMessage方法的返回类型,以支持更灵活的事件处理。