Skip to content

fix(gateway): recognize Weixin's rate-limit wording in the delivery ledger's flood classifier - #105070

Open
nftpoetrist wants to merge 1 commit into
NousResearch:mainfrom
nftpoetrist:fix/weixin-flood-ledger-classification
Open

nftpoetrist wants to merge 1 commit into
NousResearch:mainfrom
nftpoetrist:fix/weixin-flood-ledger-classification

Conversation

@nftpoetrist

@nftpoetrist nftpoetrist commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

is_flood_error() (gateway/delivery_ledger.py) only recognized Telegram's two flood shapes: the
adapters' fail-closed flood_control:<seconds> prefix, and python-telegram-bot's raw "Flood control exceeded... Retry in N seconds" text. This function gates whether a final-send failure gets a timed
redelivery armed (_schedule_flood_redelivery) and whether the boot/runtime sweeps
(sweep_recoverable, sweep_failed_for_runtime) treat a failed row as still inside its penalty window
(adopt without spending an attempt, wait for the deadline) or as an ordinary failure (claim and resend
immediately).

Weixin's rate-limit circuit breaker fails a send closed with neither shape —
"iLink sendmessage rate limited; cooldown active for {N}s" (gateway/platforms/weixin.py, a bare
SendResult(success=False, error=str(exc)), no retry_after/flood_control: framing) — so a Weixin
send throttled by iLink never got a flood timer armed. The row sat as an ordinary failed row, and the
next boot sweep or reconnect-triggered sweep claimed and resent it immediately, spending a redelivery
attempt inside the still-active cooldown instead of waiting it out — defeating the point of the
flood-ledger mechanism for this one platform.

Fix

Widened is_flood_error() to also recognize the platform-neutral rate-limit classification already
used by _send_with_retry's in-attempt backoff — classify_send_error() in
gateway/platforms/base.py (imported locally inside the new _classified_rate_limited() helper, best-
effort, to avoid a module-load-order dependency between delivery_ledger.py and the much larger
gateway/platforms/base.py) — rather than hand-rolling a second, Weixin-specific regex. This is the
same chokepoint _is_rate_limited_error() already wraps for the in-attempt backoff decision, so any
future adapter whose error text matches "flood"/"too many requests"/"retry after"/"rate limit" gets the
ledger's timed-redelivery treatment too, not just Weixin.

Also extended the wait-seconds extraction (_raw_flood_wait, used by flood_wait_seconds /
flood_not_before) with a second regex, _COOLDOWN_WAIT_RE, matching Weixin's own embedded
"cooldown active for Ns" figure — so the redelivery timer honors the platform's actual cooldown
instead of falling back to the generic 60s default every time.

_COOLDOWN_WAIT_RE = re.compile(r"cooldown active for\s+(\d+(?:\.\d+)?)s", re.IGNORECASE)

Why this approach over a scoped Weixin-only pattern

Reusing classify_send_error() was judged safer and more maintainable than adding a narrow
Weixin-specific literal match:

  • It's the same classification the send-retry path already trusts for "is this transient/rate-limited," so the ledger and the in-attempt backoff can't drift out of sync on what counts as a flood.
  • It automatically covers any other adapter whose error text says "rate limit"/"too many requests"/"retry after" without a dedicated flood_control: prefix, closing the same class of gap ahead of time rather than one platform at a time.
  • Verified no regression risk: _RETRYABLE_ERROR_PATTERNS (the separate "transient"/network-error bucket) doesn't overlap with the rate_limited classifier's patterns, and is_flood_error() is only ever called on the row's last_error string — never on "send_path_degraded" or blocked/forbidden/not-found texts, which the classifier correctly leaves unmatched (covered by a new regression assertion).

Testing

Extended tests/gateway/test_delivery_flood_invariants.py (the current, focused successor to the
older/larger flood-retry test file) with four new tests:

  • test_weixin_rate_limit_text_classifies_as_flood_and_extracts_its_own_wait — unit-level: is_flood_error() recognizes the Weixin text, flood_wait_seconds() reads the embedded 12.3 figure (not the generic default), and an ordinary (non-rate-limited) Weixin failure is correctly left unmatched.
  • test_weixin_rate_limit_arms_timed_redelivery_not_immediate_retry — end-to-end through sweep_failed_for_runtime: a Weixin flood row is not claimed while still inside its cooldown, and is claimed (with the rate-limit marker) once the deadline passes.
  • test_weixin_rate_limit_boot_adoption_does_not_spend_attempt — end-to-end through sweep_recoverable + sweep_failed_for_runtime: a dead-owner Weixin flood row is adopted without spending an attempt while cooling down, then claimed normally past the deadline (mirrors the existing Telegram boot-adoption test).
  • test_telegram_flood_detection_unaffected_by_broader_classifier — confirms Telegram's two established shapes and the "permanent rejection is not a flood" invariant are unchanged.

Ran: tests/gateway/test_delivery_flood_invariants.py, test_delivery_ledger.py, test_delivery.py,
test_weixin.py, test_restart_redelivery_dedup.py, test_send_error_classification.py — 109 passed.
ruff check on both changed files: clean.

Mutation-verified: reverted gateway/delivery_ledger.py via patch file (tests kept), reran the
four new tests — 3 of 4 failed as expected (AssertionErrors on the exact behaviors the fix adds); the
Telegram-only invariant test correctly still passed since it doesn't exercise the new code path.
Reapplied the fix — all 6 tests in the file pass again.

Competitor check

Fresh search across weixin flood / is_flood_error / delivery_ledger weixin / weixin rate limit /
flood_control weixin / delivery ledger rate limit / flood-capped / delivery_ledger, all states,
right before pushing:

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/wecom WeCom / WeChat Work adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 7, 2026
…edger's flood classifier

is_flood_error() (gateway/delivery_ledger.py) only recognized Telegram's two flood shapes: the
adapters' fail-closed "flood_control:<seconds>" prefix, and PTB's raw "Flood control exceeded...
Retry in N seconds" text. It gates whether a final-send failure gets a timed redelivery
(_schedule_flood_redelivery) and whether the boot/runtime sweeps treat a failed row as still inside
its penalty window or as an ordinary failure to retry immediately.

Weixin's rate-limit circuit breaker fails a send closed with neither shape ("iLink sendmessage rate
limited; cooldown active for {N}s", gateway/platforms/weixin.py), so a Weixin send throttled by iLink
never got a flood timer armed: the row sat as an ordinary "failed" row and the next boot/reconnect
sweep claimed and resent it immediately, spending a redelivery attempt inside the still-active
cooldown instead of waiting it out.

Widen is_flood_error() to also recognize the platform-neutral rate-limit classification already used
by _send_with_retry's in-attempt backoff (classify_send_error() in gateway/platforms/base.py, via a
local import to avoid a module-load-order dependency) instead of hand-rolling a second Weixin-specific
regex. Also extend the flood-wait extraction to read Weixin's own embedded "cooldown active for Ns"
figure (a new _COOLDOWN_WAIT_RE, tried alongside the existing PTB regex) so the redelivery timer uses
the platform's actual cooldown instead of falling back to the generic 60s default.

Extends tests/gateway/test_delivery_flood_invariants.py (mutation-verified: new tests fail without
the fix, pass with it; existing Telegram-only tests are unaffected) to cover: is_flood_error()/
flood_wait_seconds() on Weixin's error text, the runtime sweep arming a timed redelivery instead of
claiming immediately inside the cooldown, and boot-sweep adoption without spending an attempt.
@nftpoetrist
nftpoetrist force-pushed the fix/weixin-flood-ledger-classification branch from 2a9e827 to 9eba9e9 Compare September 7, 2026 17:13
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Summary

Teaches the delivery ledger's flood classifier to recognise Weixin's own rate-limit wording (cooldown active for Ns) — both extracting the embedded wait and treating classifier-recognised rate limits as floods for timed redelivery. Well-tested: Weixin timed-redelivery, boot adoption, and Telegram no-regression invariants.

Findings

  • Non-blocking — gateway/delivery_ledger.py:59-60: _raw_flood_wait now includes the generic _COOLDOWN_WAIT_RE, so any error text containing "cooldown active for Ns" counts as a flood even when the platform-neutral classifier does not recognise it as rate-limited. Slightly broader than the docstring's stated intent (classifier-gated recognition). In practice the consequence is only a timed delay rather than an immediate retry, so the failure mode is benign.
  • Non-blocking — gateway/delivery_ledger.py:38-45: classify_send_error(None, text) passes None where an error/adapter object may be expected. Any misbehavior is swallowed by the try/except → False, which is safe but could silently disable the new recognition if the classifier's signature changes. Consider a debug log in the except branch.

Verification

  • tests/gateway/test_delivery_flood_invariants.py:95-155 covers classification, wait extraction, sweep timing, and boot adoption for the Weixin shape, plus Telegram invariance. No security concerns.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/wecom WeCom / WeChat Work adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants