Fix dead retry loop in asyncio_helper._process_request#2599
Open
hsn8086 wants to merge 1 commit into
Open
Conversation
The RequestTimeout raise was inside the retry while-loop, so the first network error always aborted the call and MAX_RETRIES had no effect. Align the async helper with the opt-in retry convention of telebot.apihelper: add RETRY_ON_ERROR / RETRY_TIMEOUT module flags (default off, preserving current behaviour). When enabled, network errors are retried up to MAX_RETRIES times with RETRY_TIMEOUT seconds between attempts, and RequestTimeout is raised only after all attempts are exhausted. Errors reported by the Bot API itself (ApiTelegramException etc.) keep propagating immediately. Also covers the helper with self-contained unit tests.
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.
Description
In
asyncio_helper._process_requesttheraise RequestTimeout(...)statement sits inside the retrywhileloop:pyTelegramBotAPI/telebot/asyncio_helper.py
Lines 96 to 113 in 744e1d0
As a result, the first network error (
aiohttp.ClientError, timeout, etc.) always aborts the call immediately andMAX_RETRIEShas no effect — the loop can never reach a second iteration. Async bots therefore lose outgoing calls (send_message, ...) on any transient connection hiccup.This PR aligns the async helper with the opt-in retry convention that already exists in
telebot.apihelper(RETRY_ON_ERROR,RETRY_TIMEOUT):RETRY_ON_ERROR = False(default): behaviour is unchanged — fail fast on the first network error.RETRY_ON_ERROR = True: network errors are retried up toMAX_RETRIEStimes withRETRY_TIMEOUTseconds between attempts;RequestTimeoutis raised only after all attempts are exhausted.ApiTelegramException,ApiInvalidJSONException,ApiHTTPException) keep propagating immediately, as before.Note for reviewers: when retries are enabled, a request whose response was lost mid-flight can be repeated, i.e. an API call may be executed twice (e.g. a message sent twice). This matches the semantics of the existing sync retry engine, and is one more reason the flag stays off by default.
Describe your tests
How did you test your change?
cd tests && py.test: 58 passed, 72 skipped. Master baseline on the same machine: 53 passed, 72 skipped — the 5 extra passes are the new unit tests added here, no regressions.tests/test_asyncio_helper.py(no TOKEN needed, network stubbed): success path, default fail-fast, recovery after transient errors, attempt count ==MAX_RETRIES, immediate propagation of API errors.RETRY_TIMEOUTsleeps in between), the exhaustion path raisesRequestTimeoutafter exactlyMAX_RETRIESattempts, and the default path still fails fast with a single attempt.getMeagainst api.telegram.org with the patched module and default flags — works unchanged.Python version: 3.12.3
OS: Ubuntu Linux
Checklist:
RETRY_ON_ERROR/RETRY_TIMEOUTflags either; happy to add one if desiredRETRY_ON_ERRORdefaults toFalse, preserving the current fail-fast behaviour exactlyapihelper) already has working opt-in retries; this brings the async helper to parity