From c8f16a4e893e76d6fa4bf7acef4e1a3bce934caf Mon Sep 17 00:00:00 2001 From: "Dmitry D. Chernov" Date: Thu, 7 Feb 2019 04:41:45 +1000 Subject: [PATCH] Fix a couple of inconsistencies in the public interface (#1102) * Create `_NOT_A_REQUEST` when needed. Currently, modifications in the raised exception would be "global". * `retries` parameters were actually attempts. This has been fixed to actually be the amount of retries, so 0 now means don't retry. * Helper function to deal with retries instead of using a range with different styles every time. --- telethon/client/account.py | 2 +- telethon/client/telegrambaseclient.py | 20 ++++++++-------- telethon/client/users.py | 10 ++++---- telethon/helpers.py | 15 ++++++++++++ telethon/network/mtprotosender.py | 33 +++++++++++++++------------ 5 files changed, 50 insertions(+), 30 deletions(-) diff --git a/telethon/client/account.py b/telethon/client/account.py index 472b3f0a..dc61038e 100644 --- a/telethon/client/account.py +++ b/telethon/client/account.py @@ -53,7 +53,7 @@ class _TakeoutClient: wrapped = [] for r in requests: if not isinstance(r, TLRequest): - raise _NOT_A_REQUEST + raise _NOT_A_REQUEST() await r.resolve(self, utils) wrapped.append(functions.InvokeWithTakeoutRequest(takeout_id, r)) diff --git a/telethon/client/telegrambaseclient.py b/telethon/client/telegrambaseclient.py index 805f5ab5..2ec31df3 100644 --- a/telethon/client/telegrambaseclient.py +++ b/telethon/client/telegrambaseclient.py @@ -71,23 +71,23 @@ class TelegramBaseClient(abc.ABC): invoked requests, and you should use ``asyncio.wait`` or ``asyncio.wait_for`` for that. - request_retries (`int`, optional): + request_retries (`int` | `None`, optional): How many times a request should be retried. Request are retried when Telegram is having internal issues (due to either ``errors.ServerError`` or ``errors.RpcCallFailError``), when there is a ``errors.FloodWaitError`` less than `flood_sleep_threshold`, or when there's a migrate error. - May set to a false-y value (``0`` or ``None``) for infinite - retries, but this is not recommended, since some requests can - always trigger a call fail (such as searching for messages). + May take a negative or ``None`` value for infinite retries, but + this is not recommended, since some requests can always trigger + a call fail (such as searching for messages). - connection_retries (`int`, optional): + connection_retries (`int` | `None`, optional): How many times the reconnection should retry, either on the initial connection or when Telegram disconnects us. May be - set to a false-y value (``0`` or ``None``) for infinite - retries, but this is not recommended, since the program can - get stuck in an infinite loop. + set to a negative or ``None`` value for infinite retries, but + this is not recommended, since the program can get stuck in an + infinite loop. retry_delay (`int` | `float`, optional): The delay in seconds to sleep between automatic reconnections. @@ -236,8 +236,8 @@ class TelegramBaseClient(abc.ABC): self.api_id = int(api_id) self.api_hash = api_hash - self._request_retries = request_retries or sys.maxsize - self._connection_retries = connection_retries or sys.maxsize + self._request_retries = request_retries + self._connection_retries = connection_retries self._retry_delay = retry_delay or 0 self._proxy = proxy self._timeout = timeout diff --git a/telethon/client/users.py b/telethon/client/users.py index 44344334..609b37a9 100644 --- a/telethon/client/users.py +++ b/telethon/client/users.py @@ -6,8 +6,9 @@ from .telegrambaseclient import TelegramBaseClient from .. import errors, utils from ..errors import MultiError, RPCError from ..tl import TLObject, TLRequest, types, functions +from ..helpers import retry_range -_NOT_A_REQUEST = TypeError('You can only invoke requests, not types!') +_NOT_A_REQUEST = lambda: TypeError('You can only invoke requests, not types!') class UserMethods(TelegramBaseClient): @@ -15,7 +16,7 @@ class UserMethods(TelegramBaseClient): requests = (request if utils.is_list_like(request) else (request,)) for r in requests: if not isinstance(r, TLRequest): - raise _NOT_A_REQUEST + raise _NOT_A_REQUEST() await r.resolve(self, utils) # Avoid making the request if it's already in a flood wait @@ -34,7 +35,7 @@ class UserMethods(TelegramBaseClient): request_index = 0 self._last_request = time.time() - for _ in range(self._request_retries): + for attempt in retry_range(self._request_retries): try: future = self._sender.send(request, ordered=ordered) if isinstance(future, list): @@ -86,7 +87,8 @@ class UserMethods(TelegramBaseClient): raise await self._switch_dc(e.new_dc) - raise ValueError('Number of retries reached 0') + raise ValueError('Request was unsuccessful {} time(s)' + .format(attempt)) # region Public methods diff --git a/telethon/helpers.py b/telethon/helpers.py index 531736ce..82287647 100644 --- a/telethon/helpers.py +++ b/telethon/helpers.py @@ -72,6 +72,21 @@ def strip_text(text, entities): return text +def retry_range(retries): + """ + Generates an integer sequence starting from 1. If `retries` is + negative or ``None`` then sequence is infinite, otherwise it will + end at `retries + 1`. + """ + yield 1 + if retries is None: + retries = -1 + attempt = 0 + while attempt != retries: + attempt += 1 + yield 1 + attempt + + # endregion # region Cryptographic related utils diff --git a/telethon/network/mtprotosender.py b/telethon/network/mtprotosender.py index a756e274..cbb84971 100644 --- a/telethon/network/mtprotosender.py +++ b/telethon/network/mtprotosender.py @@ -22,6 +22,7 @@ from ..tl.types import ( MsgsStateInfo, MsgsAllInfo, MsgResendReq, upload ) from ..crypto import AuthKey +from ..helpers import retry_range def _cancellable(func): @@ -211,26 +212,27 @@ class MTProtoSender: receive loops. """ self._log.info('Connecting to %s...', self._connection) - for retry in range(1, self._retries + 1): + for attempt in retry_range(self._retries): try: - self._log.debug('Connection attempt {}...'.format(retry)) + self._log.debug('Connection attempt {}...'.format(attempt)) await self._connection.connect(timeout=self._connect_timeout) except (ConnectionError, asyncio.TimeoutError) as e: self._log.warning('Attempt {} at connecting failed: {}: {}' - .format(retry, type(e).__name__, e)) + .format(attempt, type(e).__name__, e)) await asyncio.sleep(self._delay) else: break else: - raise ConnectionError('Connection to Telegram failed {} times' - .format(self._retries)) + raise ConnectionError('Connection to Telegram failed {} time(s)' + .format(attempt)) self._log.debug('Connection success!') if not self.auth_key: plain = MTProtoPlainSender(self._connection, loggers=self._loggers) - for retry in range(1, self._retries + 1): + for attempt in retry_range(self._retries): try: - self._log.debug('New auth_key attempt {}...'.format(retry)) + self._log.debug('New auth_key attempt {}...' + .format(attempt)) self.auth_key.key, self._state.time_offset =\ await authenticator.do_authentication(plain) @@ -244,11 +246,11 @@ class MTProtoSender: break except (SecurityError, AssertionError) as e: self._log.warning('Attempt {} at new auth_key failed: {}' - .format(retry, e)) + .format(attempt, e)) await asyncio.sleep(self._delay) else: - e = ConnectionError('auth_key generation failed {} times' - .format(self._retries)) + e = ConnectionError('auth_key generation failed {} time(s)' + .format(attempt)) self._disconnect(error=e) raise e @@ -321,17 +323,17 @@ class MTProtoSender: self._state.reset() retries = self._retries if self._auto_reconnect else 0 - for retry in range(1, retries + 1): + for attempt in retry_range(retries): try: await self._connect() except (ConnectionError, asyncio.TimeoutError) as e: - self._log.info('Failed reconnection retry %d/%d with %s', - retry, retries, e.__class__.__name__) + self._log.info('Failed reconnection attempt %d with %s', + attempt, e.__class__.__name__) await asyncio.sleep(self._delay) except Exception: self._log.exception('Unexpected exception reconnecting on ' - 'retry %d/%d', retry, retries) + 'attempt %d', attempt) await asyncio.sleep(self._delay) else: @@ -343,7 +345,8 @@ class MTProtoSender: break else: - self._log.error('Failed to reconnect automatically.') + self._log.error('Automatic reconnection failed {} time(s)' + .format(attempt)) self._disconnect(error=ConnectionError()) def _start_reconnect(self):