FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Keep a RetryAfter halt until every backoff has finished by 0xSoftBoi · Pull Request #5339 · python-telegram-bot/python-telegram-bot · GitHub

Keep a RetryAfter halt until every backoff has finished - #5339

Open
0xSoftBoi wants to merge 2 commits into
python-telegram-bot:masterfrom
0xSoftBoi:fix-retry-after-halt-released-early
Open

Keep a RetryAfter halt until every backoff has finished#5339
0xSoftBoi wants to merge 2 commits into
python-telegram-bot:masterfrom
0xSoftBoi:fix-retry-after-halt-released-early

Conversation

Copy link
Copy Markdown

Closes #5338.

AIORateLimiter documents that a RetryAfter halts all requests for retry_after + 0.1 seconds. _retry_after_event is shared by the whole limiter, but it was set() in a finally that runs for every request — including requests that never cleared it. So any request already past inner()'s await self._retry_after_event.wait() when the halt began would release it on completion.

Two reachable consequences:

  • an unrelated in-flight request completing ends the halt early (0.30s into a 2s halt, in the issue's script);
  • with two requests backing off at once, the shorter backoff expiring releases the longer one.

Both need a request to be past the wait when the halt starts, which is the normal case under load — so the halt was weakest exactly when it mattered, and a 429 storm kept being fed.

Fix

Count the requests currently backing off and set the event only when that count reaches zero. A plain "only the clearer may set it" flag would fix the first case but not the second.

Tests

Two regression tests, both of which fail on master and pass here:

  • test_retry_after_not_released_by_in_flight_request
  • test_retry_after_not_released_by_shorter_backoff

They use a small ScriptedRequest helper mapping chat_id -> (latency, retry_after). The latency matters: the existing test_delay_all_pending_on_retry starts its second request after the halt is already in place, so that request blocks at the wait() and the bug is invisible. Reproducing it needs a request that is already in flight, which in turn needs the 429 to arrive after some delay, as it does in practice.

tests/ext/test_ratelimiter.py: 18 passed, 1 skipped. ruff check, ruff format --check and mypy are clean on both files.

No behaviour change other than restoring what the docstring already promises, so I have not touched the docs.

0xSoftBoi and others added 2 commits August 25, 2026 04:52
AIORateLimiter documents that a RetryAfter halts all requests for
retry_after + 0.1 seconds, but _retry_after_event was set in a finally
block that runs for every request, not just the ones that cleared it.

A request already past inner()'s wait() when the halt began would
therefore release it on completion, and a shorter backoff would release
a longer one still in effect. Under load there is nearly always another
request in flight, so the halt was weakest exactly when it mattered.

Track the number of requests currently backing off and set the event
only when that reaches zero.

Both cases are covered by regression tests; each fails without the fix.

Co-Authored-By: Claude Opus 5 (1M context) <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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIORateLimiter: a RetryAfter halt is released early by any concurrent request finishing

1 participant


Back | FazBrowse Home | New Git URL