fix: give reconnect-in-progress submissions a bounded retry window - #225
Open
YishanCoding wants to merge 1 commit into
Open
fix: give reconnect-in-progress submissions a bounded retry window#225YishanCoding wants to merge 1 commit into
YishanCoding wants to merge 1 commit into
Conversation
New submissions currently reject immediately with reconnect-in-progress whenever activeRuns.newRunsPaused() is true, and the caller's message is dropped rather than retried (see bot/channel.ts's rejectReason handling — it sends the "正在重连" notice and returns, discarding the batch). Any in-flight IM message that happens to land during a WS reconnect (a ping timeout that self-heals in a few seconds is common) is lost and the user has to notice and resend it by hand. Add RunExecutor.submit() support for waiting out a short pause before giving up: ActiveRuns gains waitForResume(timeoutMs), which resolves as soon as the pause clears or the timeout elapses, whichever comes first. RunExecutor takes an opt-in reconnectWaitMs (default 0, preserving the exact current behavior for existing callers/tests) and channel.ts wires it to 8s in production, matching the keepalive ping-timeout window so a routine reconnect blip no longer costs the user their message. nowait submissions are unaffected — they've already asked not to wait. No behavior change when reconnectWaitMs is left unset.
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
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.
Fixes #224
Problem
RunExecutor.submit()rejects immediately withreconnect-in-progressany timeactiveRuns.newRunsPaused()is true.bot/channel.ts's IM flush handler treats that rejection as terminal — it sends the "当前 bot 正在重连,稍后会继续处理新消息。" notice and discards the message, with no retry. Since reconnect windows are frequently just a few seconds (a WS ping timeout that self-heals — see the keepalive's 15s ping-timeout logic), any message unlucky enough to land in that window is silently lost rather than actually being handled "稍后" as the notice claims.Fix
ActiveRunsgainswaitForResume(timeoutMs): Promise<boolean>, resolvingtrueas soon as the current pause is released (all nestedpauseNewRunscalls have unwound) orfalseoncetimeoutMselapses first. Implemented with a small waiter list notified from the existingpauseNewRunsrelease callback — no polling.RunExecutortakes a new optionalreconnectWaitMs(default0). When> 0and the submission isn'tnowait, a pausedsubmit()call waits up to that long for the pause to clear before falling back to the originalreconnect-in-progressrejection.nowaitsubmissions are unaffected — they already opt out of waiting for anything (pool included).bot/channel.tswiresreconnectWaitMs: 8000for the productionRunExecutor, matching the keepalive's ping-timeout window.Compatibility
reconnectWaitMsdefaults to0, which reproduces the exact prior behavior (synchronous, immediate rejection) — every existing test intests/integration/executor/run-executor.test.tsandtests/unit/runtime/run-executor.test.tspasses unmodified. This is purely additive/opt-in.Testing
tests/unit/bot/active-runs.test.ts(6 cases — resolves immediately when unpaused, resolves on resume before timeout, resolves false on timeout while still paused, non-positive timeout, nested pause/resume only fires once fully unpaused, no waiter leak across cycles).tests/integration/executor/run-executor.test.ts(4 cases — holds and then runs a submission once a short pause clears, still rejects once the wait window elapses,nowaitfails fast even withreconnectWaitMsconfigured, and the default/unset case keeps the immediate-reject behavior).pnpm test:unit,pnpm test:integration,pnpm typecheck,pnpm buildall pass. (Pre-existing failures intests/unit/observability/logger.test.tsand threetests/integration/**files are unrelated — they hardcode the log filenamebridge-20260525.jsonland fail on any other date; confirmed unchanged before/after this change.)