fix(): dispatch onMessageClose only after retry limit exceeded #4565
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.spec.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 (
|
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 | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
16
|
| View all changes introduced in this branch ↗︎ | |
![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 (2)
packages/easyops-runtime/src/websocket/MessageService.spec.ts (1)
56-56: 测试用例修改符合预期行为此处修改正确反映了新的 WebSocket 错误处理逻辑，确保在重试限制未超过时不会触发 onClose 回调。
建议添加以下测试场景：
- 验证达到重试限制后 onClose 被正确调用
- 验证重试次数计数是否准确
- 验证不同的 WebSocket 错误类型的处理
示例测试用例：
test("should call onClose after retry limit exceeded", async () => { const onClose = jest.fn(); client.onClose(onClose); // 模拟多次连接失败直至超过重试限制 for (let i = 0; i <= retryLimit; i++) { await server.connected; server.error({ code: 1001, reason: "test", wasClean: false, }); await new Promise((resolve) => setTimeout(resolve, 1500)); } expect(onClose).toBeCalledTimes(1); });packages/easyops-runtime/src/websocket/MessageService.ts (1)
75-78: 代码变更正确实现了重试限制后的关闭事件通知这个改动符合预期行为，确保只有在超过重试限制后才会触发关闭监听器。这样可以避免在临时断开连接时过早通知监听器，提高了连接恢复的可靠性。
建议在触发关闭监听器之前添加更详细的日志记录，以便更好地进行问题诊断。例如：
console.error("WebSocket connect retry limit exceeded"); + console.log("Notifying close listeners after retry limit exceeded"); // Keep behavior as v2: emit close event only after retry limit exceeded for (const listener of this.#closeListeners) { listener(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/easyops-runtime/src/websocket/MessageDispatcher.spec.ts(1 hunks)packages/easyops-runtime/src/websocket/MessageService.spec.ts(1 hunks)packages/easyops-runtime/src/websocket/MessageService.ts(1 hunks)
🔇 Additional comments (2)
packages/easyops-runtime/src/websocket/MessageService.ts (1)
75-78: 建议验证以下重连场景
为确保改动的正确性和稳定性，建议验证以下场景：
- 临时网络中断：确认在重试限制内不会触发关闭监听器
- 永久连接失败：确认在超过重试限制后正确触发关闭监听器
✅ Verification successful
重连和关闭事件处理逻辑已验证完成
根据代码实现，确认以下行为：
- 在重试限制内的临时断开不会触发关闭监听器，而是会自动重连
- 重连逻辑采用指数退避策略（2^retryCount * RETRY_TIMEOUT_UNIT）
- 只有在超过重试限制或显式调用 close() 时才会触发关闭监听器
代码实现符合预期要求，保持了与 v2 版本一致的行为。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索相关测试用例，确保覆盖了这些场景
rg -A 10 "describe.*MessageService.*reconnect"
Length of output: 48
Script:
#!/bin/bash
# Search for test files related to MessageService
fd -e test.ts -e spec.ts MessageService
# Search for reconnection and close event handling tests
rg -A 10 "test.*reconnect|test.*close.*event" packages
# Search for MessageService implementation to understand reconnection logic
ast-grep --pattern 'class MessageService {
$$$
reconnect($$$) {
$$$
}
$$$
}'
Length of output: 27551
Script:
#!/bin/bash
# Search for MessageService implementation and reconnection logic
rg -A 20 "class MessageService" packages/easyops-runtime/src/websocket/MessageService.ts
# Search for retry and close event handling
rg -A 10 "retryLimit|closeListeners" packages/easyops-runtime/src/websocket/MessageService.ts
# Search for test cases related to close events
rg -A 10 "test.*close.*event|test.*retry" packages/easyops-runtime/src/websocket/MessageService.spec.ts
Length of output: 1859
packages/easyops-runtime/src/websocket/MessageDispatcher.spec.ts (1)
140-140: 测试用例更新符合新的关闭事件处理逻辑
这个更改反映了 WebSocket 关闭事件处理的重要调整。现在期望在简单的服务器关闭时不触发 onClose 回调，这与 PR 的目标一致：只在重试限制超过后才触发 onMessageClose。
这个测试用例的修改很好地验证了这个新行为，确保了消息调度器在正常关闭时不会过早触发关闭回调。
执行以下脚本来验证相关的更改：
✅ Verification successful
测试用例更改正确反映了 WebSocket 重试机制的行为
根据代码分析，这个测试用例的修改是正确的。在 MessageService 的实现中，只有在重试次数超过限制（RETRY_LIMIT）后才会触发 onClose 回调：
if (this.#retryCount >= RETRY_LIMIT) {
console.error("WebSocket connect retry limit exceeded");
// Keep behavior as v2: emit close event only after retry limit exceeded
for (const listener of this.#closeListeners) {
listener();
}
return;
}测试用例中期望 onClose 不被调用是符合这个实现的，因为：
- 在简单的连接断开时，系统会自动尝试重连
- 只有在重试次数超过限制后才会触发 onClose 事件
- 测试场景中并未触发重试限制超出的情况
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查 MessageService 中是否有对应的实现更改
# 搜索 MessageService 中的 onClose 相关代码
ast-grep --pattern 'onClose($$$)'
# 搜索重试限制相关的代码
rg -A 5 'retryLimit|maxRetries'
Length of output: 69
Script:
#!/bin/bash
# 搜索 MessageDispatcher 和相关文件中的 onClose 处理逻辑
rg -l "onClose" packages/easyops-runtime/src/websocket/
# 显示 MessageDispatcher.ts 的完整内容以分析实现
cat packages/easyops-runtime/src/websocket/MessageDispatcher.ts
# 搜索与重试和关闭相关的代码片段
rg -A 5 "onClose|retry|disconnect" packages/easyops-runtime/src/websocket/
Length of output: 13352
依赖检查
组件之间的依赖声明，是微服务组件架构下的重要信息，请确保其正确性。
请勾选以下两组选项其中之一：
或者：
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG，请检查工作内容与提交信息是否相符，并在以下每组选项中都依次确认。
破坏性变更：
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性：
feat作为提交类型。问题修复：
fix作为提交类型。杂项工作：
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动，例如修改注释、测试用例、开发文档等：
chore,docs,test等作为提交类型。Summary by CodeRabbit
Bug Fixes
MessageDispatcher和MessageService类的onClose回调的期望行为，确保在特定条件下不会触发该回调。Tests
onClose回调的新期望和错误处理逻辑。