fix(channels): add circuit breaker to typing keepalive loop#27593
fix(channels): add circuit breaker to typing keepalive loop#27593xilanhua12138 wants to merge 1 commit intoopenclaw:mainfrom
Conversation
When the typing indicator start() call fails (e.g., message deleted, API errors), the keepalive loop previously continued firing indefinitely. This could trigger API rate limiting (e.g., Feishu 429) by sending requests every 3 seconds for a resource that no longer exists. Add a consecutive failure counter that stops the keepalive loop after 2 consecutive failures (configurable via maxConsecutiveFailures). The counter resets on any successful call or when a new reply starts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21b913b366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| consecutiveFailures = 0; | ||
| keepaliveLoop.stop(); | ||
| await fireStart(); | ||
| keepaliveLoop.start(); |
There was a problem hiding this comment.
Skip keepalive restart after breaker trips on first failure
When maxConsecutiveFailures is configured to 1 (or any threshold reached by the initial fireStart()), fireStart() can trip the breaker but onReplyStart() still unconditionally calls keepaliveLoop.start(). That restarts periodic retries even though the breaker condition has already been met, so the new circuit-breaker contract is violated for this input and failing channels continue issuing requests until a later tick stops them again.
Useful? React with 👍 / 👎.
Greptile SummaryAdded circuit breaker to Key changes:
Confidence Score: 5/5
Last reviewed commit: 21b913b |
|
Superseded by main commit 37a138c. Closed in favor of the integrated cluster landing. The final merge includes:
Resolved issues in the unified landing:
Thank you; the circuit-breaker direction was incorporated and tightened in the final merged patch. |
Summary
When the typing indicator
start()call fails (e.g., the target message has been deleted or the API returns an error), the keepalive loop previously continued firing requests every 3 seconds indefinitely. This caused cascading issues:This PR adds a circuit breaker to
createTypingCallbacks:start()failuresmaxConsecutiveFailures)Changes
src/channels/typing.ts— AddedconsecutiveFailurescounter andmaxConsecutiveFailuresoption (default: 2). The keepalive loop is stopped when the threshold is reached.src/channels/typing.test.ts— Added 3 test cases:Test plan
pnpm vitest run src/channels/typing.test.ts