Repository navigation
fix(gateway): honour Telegram flood waits on streaming edits - #105340
AlexxRussell wants to merge 1 commit into
Conversation
A flood-refused preview edit was treated as a generic failure: double the edit interval, count a strike, give up after three. The wait Telegram had already put on the result was never read. Starting from a sub-second interval, three doublings still land far below any real penalty, so all three strikes were spent within half a second, and every refused edit was itself another request against the same limit. The preview froze mid-render showing raw MarkdownV2, the gateway sent the complete reply separately into a penalty that had meanwhile escalated, and the delivery ledger redelivered it minutes later. A refusal carrying a positive retry_after within a cap now sets a pause deadline instead of a strike. Interim ticks are gated while it holds and keep accumulating, so the first edit afterwards carries everything buffered. The first compliant refusal costs nothing; one arriving after an honoured deadline counts, so three refusals despite compliance still fall back. A missing, invalid or over-cap wait keeps the previous strike and doubling behaviour unchanged. A turn-final edit refused with a bounded wait is retried once after it, since the fallback send would go into the same ban. The cursor-stuck delivery rule and the immediate-fallback adapter opt-in are untouched. Waiting needs more than the 5s the gateway gives the consumer task, so the consumer publishes the outstanding wait and _await_stream_task adds it to that budget. Cancellation keeps propagating: the owner of the timeout extends it rather than the consumer swallowing a cancel to outlive it. A cancel that does arrive is a real abort, and teardown takes the edit it can get at once rather than sitting out a ban nobody is waiting for.
alt-glitch
added
type/bug
SummaryMakes the gateway honor Telegram flood waits on streaming edits: Findings
VerdictLooks good. Careful cancel semantics, bounded waits, documented residual. Safe to merge. |
|
On the third note, the residual case: confirmed, and it is the path that actually ran in the incident above. A final send refused by flood control is classified in So when a penalty begins only after the join has started, the turn cancels at 5s and falls back to the fresh send exactly as it did before this change, and if that send is refused too the ledger owns delivery from there. The reply is late, never missing. One nuance worth stating, since it is why I was comfortable leaving the residual rather than chasing it: the redelivery is a fresh send, so it carries no streamed preview identity. Following it with preview cleanup would need the preview ids carried on the obligation row, which is a separate change I have deliberately kept out of this PR. |
|
Note on how this relates to #105200, since the two came out of the same incident and could look overlapping. #105340 fixes the cause. A flood-refused preview edit ignored the #105200 fixes the leftover. On the one path the consumer does not own, where the gateway sends the final itself, nothing deleted the frozen preview, so it stayed above the real answer showing raw MarkdownV2 and the streaming cursor. They are complementary rather than alternatives, and neither depends on the other. #105340 makes the abandoned-preview path much rarer without removing it: a penalty above the cap, an adapter that reports no Two files appear in both, Happy to combine them into one PR if that is easier to review, or to rebase either onto a newer main on request. |
…nstead of re-striking inside the penalty `_on_edit_failure` treated a flood-refused edit as a generic failure: a strike plus `min(interval * 2, 10)`. Starting from the 0.8s default that is 1.6s then 3.2s, so all three strikes (and three more refused requests, each extending the ban) were spent in about five seconds of a penalty Telegram had already told us is 9s or longer, and edits were then abandoned for the rest of the turn. - The interim interval now becomes `max(doubling, retry_after)` (capped at 30s; interim edits are skipped, not slept, so a long wait only costs a stale preview). - `_should_edit`'s `buffer_threshold` clause no longer overrides an active flood backoff: once the reply passed 24 codepoints every 50ms tick re-edited regardless of the interval, which made both the legacy doubling and any server wait dead letters. Slim redo of the retry_after half of #105340 (analysis by @AlexxRussell on #116312); the pause/join-budget machinery there is not needed once the interval itself is honoured.
…nstead of re-striking inside the penalty `_on_edit_failure` treated a flood-refused edit as a generic failure: a strike plus `min(interval * 2, 10)`. Starting from the 0.8s default that is 1.6s then 3.2s, so all three strikes (and three more refused requests, each extending the ban) were spent in about five seconds of a penalty Telegram had already told us is 9s or longer, and edits were then abandoned for the rest of the turn. - The interim interval now becomes `max(doubling, retry_after)` (capped at 30s; interim edits are skipped, not slept, so a long wait only costs a stale preview). - `_should_edit`'s `buffer_threshold` clause no longer overrides an active flood backoff: once the reply passed 24 codepoints every 50ms tick re-edited regardless of the interval, which made both the legacy doubling and any server wait dead letters. Slim redo of the retry_after half of #105340 (analysis by @AlexxRussell on #116312); the pause/join-budget machinery there is not needed once the interval itself is honoured.
|
Superseded by #116987 (merged as The half that mattered is now in main: The pause and join-budget machinery here existed to stop the 50ms tick from I said on #116987 that I would close this once it landed, so here that is. |
…nstead of re-striking inside the penalty `_on_edit_failure` treated a flood-refused edit as a generic failure: a strike plus `min(interval * 2, 10)`. Starting from the 0.8s default that is 1.6s then 3.2s, so all three strikes (and three more refused requests, each extending the ban) were spent in about five seconds of a penalty Telegram had already told us is 9s or longer, and edits were then abandoned for the rest of the turn. - The interim interval now becomes `max(doubling, retry_after)` (capped at 30s; interim edits are skipped, not slept, so a long wait only costs a stale preview). - `_should_edit`'s `buffer_threshold` clause no longer overrides an active flood backoff: once the reply passed 24 codepoints every 50ms tick re-edited regardless of the interval, which made both the legacy doubling and any server wait dead letters. Slim redo of the retry_after half of NousResearch#105340 (analysis by @AlexxRussell on NousResearch#116312); the pause/join-budget machinery there is not needed once the interval itself is honoured. (cherry picked from commit 75d92e3)
3 participants
A streaming preview is abandoned mid-render whenever Telegram asks the gateway to wait, because the consumer never reads the wait it was given. The reader is left with a truncated raw-markdown message, and the complete answer arrives separately minutes later.
What happens
_on_edit_failureingateway/stream_consumer_transport.pytreats a flood-refused preview edit as a generic failure: it doubles_current_edit_intervaland counts a strike, giving up after_MAX_FLOOD_STRIKES. It ignoresretry_after, which the adapter has already put on the result (_flood_cap_resultreturnserror="flood_control:<secs>"withretry_afterset whenever the wait exceeds the adapter's 5 second inline cap).Because the edit interval starts well under a second, doubling it three times still lands far below any real penalty, so all three strikes are spent almost immediately. Each refused edit is itself another request against the same limit, so complying is replaced by hammering.
Observed
On a 1 GB deployment running the Telegram adapter, 7 September 2026:
Four preview edits were refused inside 0.4 seconds, each asking for 9 seconds. The preview froze mid-sentence showing raw MarkdownV2, because a partial preview is rendered without it. The 9 second penalty had escalated to 271 seconds by the time the gateway sent the complete reply itself, so that send was refused too and the delivery ledger redelivered it 4.5 minutes after the question. The same pattern appears twice that day and six times since 25 May.
The change
Honour the wait. A flood refusal carrying a positive
retry_afterwithin a cap sets a monotonic pause deadline instead of a strike. Interim ticks are gated while the pause is active and keep accumulating, so the first edit after it carries everything buffered. A successful edit clears the pause and the strikes.Keep the runaway bounded. The first compliant refusal costs no strike. A refusal arriving after an honoured deadline has elapsed does count, so three refusals despite compliance still fall back. A missing, invalid, non-positive or over-cap wait keeps the existing strike and interval doubling byte for byte.
Retry the final edit once. A turn-final edit refused with a bounded wait is retried after it, because the fallback send would go into the same ban. A second refusal takes the existing fallback path. The cursor-stuck delivery rule and the
FALLBACK_ON_FINAL_EDIT_FLOODadapter opt-in are unchanged.Let the gateway grant the wait.
_await_stream_taskjoins the consumer task for 5 seconds. A consumer sitting out a penalty needs longer, so the consumer publishesflood_pause_remainingand the gateway adds it to that budget. Cancellation therefore still propagates: rather than the consumer swallowing a cancel to outlive a timeout it does not own, the owner of the timeout extends it. A cancel that does arrive is a real abort, and teardown takes the edit it can get immediately rather than sitting out a ban nobody is waiting for. A penalty that only begins after the join has started still cancels at 5 seconds and falls back, as before.The cap is a consumer config field,
flood_pause_cap_seconds, defaulting to 30 seconds. It is not wired to user config, matching the other timing fields on that dataclass.Tests
tests/gateway/test_stream_edit_flood_pause.py, 23 cases driving the real consumer with refusedSendResults and a controllable monotonic clock. Nothing sleeps in real time. 17 fail without the change.They pin the pause and the coalesced resume, the cap boundary, the legacy path for an unusable wait, bounded repeated refusal, the single final retry and its fallback, segment finalization and tail flushing, the success reset, cursor accounting, a stale turn during the wait, that a cancel aborts instead of being swallowed, the published remaining wait, and the gateway's extended join budget.
One existing case changes:
test_non_opt_in_adapter_keeps_adaptive_final_edit_retryasserted a strike on the first refusal and no fallback. A non-opt-in adapter now waits out the penalty and retries once, so the strike lands on the second refusal and the fallback arms after it. Its intent, that immediate fallback stays scoped to opted-in adapters, is unchanged. The test also no longer sleeps for the 30 seconds it was asking for.Full gateway suite: no failures introduced.
Scope
The consumer and its transport, plus the join budget in
gateway/run_turn.py. No adapter changes, no change to the adapter's 5 second inline cap, and nothing in the delivery ledger. Fresh-final, draft and native streaming keep their existing selection and delivery behaviour, with the pause enforced before delivery. No dependency or user-config schema changes.