Skip to content

Commit bfa90bd

Browse files
committed
Fix dead retry loop in asyncio_helper._process_request
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.
1 parent 744e1d0 commit bfa90bd

2 files changed

Lines changed: 148 additions & 13 deletions

File tree

telebot/asyncio_helper.py

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import asyncio # for future uses
1+
import asyncio
22
import ssl
33
import threading
44
import aiohttp
@@ -26,6 +26,11 @@
2626

2727
REQUEST_TIMEOUT = 300
2828
MAX_RETRIES = 3
29+
# Retrying on network errors is opt-in, mirroring telebot.apihelper:
30+
# when RETRY_ON_ERROR is True, requests failing with a network error are
31+
# repeated up to MAX_RETRIES times with RETRY_TIMEOUT seconds in between.
32+
RETRY_ON_ERROR = False
33+
RETRY_TIMEOUT = 2
2934

3035
REQUEST_LIMIT = 50
3136

@@ -90,29 +95,30 @@ async def _process_request(token, url, method='get', params=None, files=None, **
9095
params = _prepare_data(params, files)
9196

9297
timeout = aiohttp.ClientTimeout(total=request_timeout)
93-
got_result = False
94-
current_try=0
98+
current_try = 0
99+
max_tries = MAX_RETRIES if RETRY_ON_ERROR else 1
95100
session = await session_manager.get_session()
96-
while not got_result and current_try<MAX_RETRIES-1:
97-
current_try +=1
101+
while current_try < max_tries:
102+
current_try += 1
98103
try:
99104
async with session.request(method=method, url=API_URL.format(token, url), data=params, timeout=timeout, proxy=proxy) as resp:
100-
got_result = True
101105
logger.debug("Request: method={0} url={1} params={2} files={3} request_timeout={4} current_try={5}".format(method, url, params, files, request_timeout, current_try).replace(token, token.split(':')[0] + ":{TOKEN}"))
102-
106+
103107
json_result = await _check_result(url, resp)
104108
if json_result:
105109
return json_result['result']
110+
return None
106111
except (ApiTelegramException,ApiInvalidJSONException, ApiHTTPException) as e:
107112
raise e
108113
except aiohttp.ClientError as e:
109-
logger.error('Aiohttp ClientError: {0}'.format(e.__class__.__name__))
114+
logger.error('Aiohttp ClientError: {0} (try #{1})'.format(e.__class__.__name__, current_try))
110115
except Exception as e:
111-
logger.error(f'Unknown error: {e.__class__.__name__}')
112-
if not got_result:
113-
raise RequestTimeout("Request timeout. Request: method={0} url={1} params={2} files={3} request_timeout={4}".format(method, url, params, files, request_timeout, current_try))
114-
return None
115-
116+
logger.error('Unknown error: {0} (try #{1})'.format(e.__class__.__name__, current_try))
117+
if current_try < max_tries:
118+
await asyncio.sleep(RETRY_TIMEOUT)
119+
raise RequestTimeout("Request timeout. Request: method={0} url={1} params={2} files={3} request_timeout={4}".format(method, url, params, files, request_timeout))
120+
121+
116122
def _prepare_file(obj):
117123
"""
118124
Prepares file for upload.

tests/test_asyncio_helper.py

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
# -*- coding: utf-8 -*-
2+
"""Unit tests for `telebot.asyncio_helper._process_request` retry behaviour.
3+
4+
These tests are self-contained (no TOKEN required) and stub out all
5+
network I/O.
6+
"""
7+
import asyncio
8+
9+
import aiohttp
10+
import pytest
11+
12+
from telebot import asyncio_helper
13+
14+
15+
class _FakeResponse:
16+
def __init__(self, payload):
17+
self._payload = payload
18+
self.status = 200
19+
20+
async def json(self, encoding=None):
21+
return self._payload
22+
23+
24+
class _FakeRequestContext:
25+
def __init__(self, outcome):
26+
self._outcome = outcome
27+
28+
async def __aenter__(self):
29+
if isinstance(self._outcome, Exception):
30+
raise self._outcome
31+
return self._outcome
32+
33+
async def __aexit__(self, exc_type, exc, tb):
34+
return False
35+
36+
37+
class _FakeSession:
38+
"""Yields the given outcomes (exception or response) one per request."""
39+
40+
def __init__(self, outcomes):
41+
self._outcomes = list(outcomes)
42+
self.calls = 0
43+
44+
def request(self, **kwargs):
45+
outcome = self._outcomes[min(self.calls, len(self._outcomes) - 1)]
46+
self.calls += 1
47+
return _FakeRequestContext(outcome)
48+
49+
50+
def _ok_response(result=42):
51+
return _FakeResponse({"ok": True, "result": result})
52+
53+
54+
def _invoke(outcomes, retry_on_error):
55+
"""Run _process_request against stubbed outcomes.
56+
57+
Returns (result, raised_exception, fake_session).
58+
"""
59+
session = _FakeSession(outcomes)
60+
61+
async def fake_get_session():
62+
return session
63+
64+
saved_get_session = asyncio_helper.session_manager.get_session
65+
saved_retry_on_error = asyncio_helper.RETRY_ON_ERROR
66+
saved_retry_timeout = asyncio_helper.RETRY_TIMEOUT
67+
asyncio_helper.session_manager.get_session = fake_get_session
68+
asyncio_helper.RETRY_ON_ERROR = retry_on_error
69+
asyncio_helper.RETRY_TIMEOUT = 0
70+
try:
71+
result = asyncio.run(
72+
asyncio_helper._process_request("1:fake", "sendMessage", method="post")
73+
)
74+
return result, None, session
75+
except Exception as exc:
76+
return None, exc, session
77+
finally:
78+
asyncio_helper.session_manager.get_session = saved_get_session
79+
asyncio_helper.RETRY_ON_ERROR = saved_retry_on_error
80+
asyncio_helper.RETRY_TIMEOUT = saved_retry_timeout
81+
82+
83+
def test_success_returns_result_payload():
84+
result, exc, session = _invoke([_ok_response()], retry_on_error=False)
85+
assert exc is None
86+
assert result == 42
87+
assert session.calls == 1
88+
89+
90+
def test_network_error_is_not_retried_by_default():
91+
"""Default behaviour stays unchanged: fail fast on the first error."""
92+
result, exc, session = _invoke(
93+
[aiohttp.ClientConnectionError("boom"), _ok_response()],
94+
retry_on_error=False,
95+
)
96+
assert isinstance(exc, asyncio_helper.RequestTimeout)
97+
assert session.calls == 1
98+
99+
100+
def test_retries_recover_when_enabled():
101+
result, exc, session = _invoke(
102+
[
103+
aiohttp.ClientConnectionError("boom"),
104+
asyncio.TimeoutError(),
105+
_ok_response(),
106+
],
107+
retry_on_error=True,
108+
)
109+
assert exc is None
110+
assert result == 42
111+
assert session.calls == 3
112+
113+
114+
def test_retries_exhausted_raise_request_timeout():
115+
result, exc, session = _invoke(
116+
[aiohttp.ClientConnectionError("boom")], retry_on_error=True
117+
)
118+
assert isinstance(exc, asyncio_helper.RequestTimeout)
119+
assert session.calls == asyncio_helper.MAX_RETRIES
120+
121+
122+
def test_api_error_is_not_retried():
123+
"""Errors reported by the Bot API itself must propagate immediately."""
124+
bad_request = _FakeResponse(
125+
{"ok": False, "error_code": 400, "description": "Bad Request: test"}
126+
)
127+
result, exc, session = _invoke([bad_request, _ok_response()], retry_on_error=True)
128+
assert isinstance(exc, asyncio_helper.ApiTelegramException)
129+
assert session.calls == 1

0 commit comments

Comments
 (0)