Repository navigation
fix(client-runtime): a busy thread stream no longer runs out of retries - #230
Merged
Merged
Conversation
The fork's bounded retry for expected subscription failures (for example the server's "live event buffer is full") meant to reset its budget whenever a value arrived, but each retry computed its attempt from the attempt the stream was started with. After the policy's last delay (7 failures for threads), the subscription went dormant for the rest of the WebSocket session even if values arrived between the failures, leaving a busy thread stale until reconnect. Each failure now reads the attempt from the shared retry state, which a delivered value resets to 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
size:S
vouch:trusted
Review found that resetting on every delivered value let a stream that fails right after each value retry forever at the shortest delay. The budget now resets only when an attempt stayed healthy for 10 s (longer than the longest retry delay) after its first value; tests cover both cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Thread and shell subscriptions retry "expected" failures in the same WebSocket session with a bounded policy (
threadSubscriptionRetryDelay: 250 ms, 500 ms, 1 s, 2 s, 4 s, 8 s). The most common one is the server'sThe live event buffer is full, which a busy thread hits whenever the client falls behind. The fork's code inpackages/client-runtime/src/rpc/client.tsmeant to reset the budget whenever a value arrives, and it did writeattempt: 0toretryState. But every retry computed its next attempt fromretryAttempt, the value the stream started with, so the reset was ignored.Result: after the 7th failure in one session, even with values arriving in between, the subscription went dormant. A busy running thread then sat on "The live event buffer is full…" until the socket reconnected. With desktop keep-alive, running threads stay mounted, so they can stay stale for the whole session.
Found by the client-sync analysis subagent.
Fix
Inside the existing
T3-CUSTOM(expbkt3)block, each subscription attempt records when it first delivered a value. On an expected failure, the budget resets to 0 only if the attempt stayed healthy for 10 s after that first value (RETRY_BUDGET_RESET_AFTER_MS, longer than the longest 8 s retry delay); otherwise the attempt keeps counting. Delays, the dormant flag and the next subscription are unchanged.A review by a fresh-context subagent caught that resetting on every value, the first version of this PR, would let a stream that fails right after each value retry forever at 250 ms.
Evidence
rpc/client.retryBudget.expbkt3.test.ts, with a policy that allows only 2 consecutive retries:rpc/client.test.ts: all pass (15 together with the new tests).state/threads-sync.test.ts+shell-sync.test.ts: 34 pass.vp run typecheckinpackages/client-runtimeis clean, and the fork-marker check passes.Reaches users: browser users on the next bkmain deploy; desktop users with the next desktop build (the app bundles its web UI).
Model/harness: Claude Opus 5.5 via Claude Code in T3 Code.
🤖 Generated with Claude Code