From da11561f87cb7a81000fdd6c9a2a114136d85e14 Mon Sep 17 00:00:00 2001 From: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com> Date: Mon, 27 Nov 2023 18:24:21 +0100 Subject: [PATCH] Adjust `read_timeout` Behavior for `Bot.get_updates` (#3963) --- docs/auxil/kwargs_insertion.py | 15 ++++--- docs/auxil/sphinx_hooks.py | 16 ++----- telegram/_bot.py | 22 ++++++++-- telegram/ext/_application.py | 33 ++++++++++++++- telegram/ext/_extbot.py | 2 +- telegram/ext/_updater.py | 63 ++++++++++++++++++++++------ telegram/request/_baserequest.py | 18 ++++++++ telegram/request/_httpxrequest.py | 10 +++++ tests/ext/test_application.py | 51 +++++++++++++++++++++- tests/ext/test_applicationbuilder.py | 31 ++++++++++++++ tests/ext/test_updater.py | 35 +++++++++++++++- tests/request/test_request.py | 18 ++++++++ tests/test_bot.py | 60 +++++++++++++++++++++++++- 13 files changed, 334 insertions(+), 40 deletions(-) diff --git a/docs/auxil/kwargs_insertion.py b/docs/auxil/kwargs_insertion.py index 4235f9e0ae4..0ccf2dfa438 100644 --- a/docs/auxil/kwargs_insertion.py +++ b/docs/auxil/kwargs_insertion.py @@ -20,9 +20,9 @@ keyword_args = [ "Keyword Arguments:", ( - " read_timeout ({read_timeout_type}, optional): Value to pass to " + " read_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to " " :paramref:`telegram.request.BaseRequest.post.read_timeout`. Defaults to " - " {read_timeout}." + " :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. " ), ( " write_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to " @@ -73,11 +73,14 @@ "", "", ] -read_timeout_sub = [ - ":attr:`~telegram.request.BaseRequest.DEFAULT_NONE`", - "``2``. :paramref:`timeout` will be added to this value", +get_updates_read_timeout_addition = [ + " :paramref:`timeout` will be added to this value.", + "", + "", + " .. versionchanged:: NEXT.VERSION", + " Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE` instead of ", + " ``2``.", ] -read_timeout_type = [":obj:`float` | :obj:`None`", ":obj:`float`"] def find_insert_pos_for_kwargs(lines: list[str]) -> int: diff --git a/docs/auxil/sphinx_hooks.py b/docs/auxil/sphinx_hooks.py index f8b4c04d20e..3748000f313 100644 --- a/docs/auxil/sphinx_hooks.py +++ b/docs/auxil/sphinx_hooks.py @@ -29,11 +29,10 @@ from docs.auxil.kwargs_insertion import ( check_timeout_and_api_kwargs_presence, find_insert_pos_for_kwargs, + get_updates_read_timeout_addition, keyword_args, media_write_timeout_deprecation, media_write_timeout_deprecation_methods, - read_timeout_sub, - read_timeout_type, ) from docs.auxil.link_code import LINE_NUMBERS @@ -107,7 +106,7 @@ def autodoc_process_docstring( f"Couldn't find the correct position to insert the keyword args for {obj}." ) - get_updates_sub = 1 if (method_name == "get_updates") else 0 + get_updates: bool = method_name == "get_updates" # The below can be done in 1 line with itertools.chain, but this must be modified in-place insert_idx = insert_index for i in range(insert_index, insert_index + len(keyword_args)): @@ -118,18 +117,11 @@ def autodoc_process_docstring( and method_name in media_write_timeout_deprecation_methods ): effective_insert: list[str] = media_write_timeout_deprecation + elif get_updates and to_insert.lstrip().startswith("read_timeout"): + effective_insert = [to_insert] + get_updates_read_timeout_addition else: effective_insert = [to_insert] - effective_insert = [ - entry.format( - method=method_name, - read_timeout=read_timeout_sub[get_updates_sub], - read_timeout_type=read_timeout_type[get_updates_sub], - ) - for entry in effective_insert - ] - lines[insert_idx:insert_idx] = effective_insert insert_idx += len(effective_insert) diff --git a/telegram/_bot.py b/telegram/_bot.py index d269196b7e0..6d06f32e3b6 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -101,7 +101,7 @@ from telegram.request import BaseRequest, RequestData from telegram.request._httpxrequest import HTTPXRequest from telegram.request._requestparameter import RequestParameter -from telegram.warnings import PTBUserWarning +from telegram.warnings import PTBDeprecationWarning, PTBUserWarning if TYPE_CHECKING: from telegram import ( @@ -3496,7 +3496,7 @@ async def get_updates( timeout: Optional[int] = None, allowed_updates: Optional[Sequence[str]] = None, *, - read_timeout: float = 2, + read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3558,6 +3558,22 @@ async def get_updates( "allowed_updates": allowed_updates, } + # The "or 0" is needed for the case where read_timeout is None. + if not isinstance(read_timeout, DefaultValue): + arg_read_timeout: float = read_timeout or 0 + else: + try: + arg_read_timeout = self._request[0].read_timeout or 0 + except NotImplementedError: + arg_read_timeout = 2 + self._warn( + f"The class {self._request[0].__class__.__name__} does not override " + "the property `read_timeout`. Overriding this property will be mandatory in " + "future versions. Using 2 seconds as fallback.", + PTBDeprecationWarning, + stacklevel=3, + ) + # Ideally we'd use an aggressive read timeout for the polling. However, # * Short polling should return within 2 seconds. # * Long polling poses a different problem: the connection might have been dropped while @@ -3568,7 +3584,7 @@ async def get_updates( await self._post( "getUpdates", data, - read_timeout=read_timeout + timeout if timeout else read_timeout, + read_timeout=arg_read_timeout + timeout if timeout else arg_read_timeout, write_timeout=write_timeout, connect_timeout=connect_timeout, pool_timeout=pool_timeout, diff --git a/telegram/ext/_application.py b/telegram/ext/_application.py index 5cc4b72c435..3bb743cd195 100644 --- a/telegram/ext/_application.py +++ b/telegram/ext/_application.py @@ -700,7 +700,7 @@ def run_polling( poll_interval: float = 0.0, timeout: int = 10, bootstrap_retries: int = -1, - read_timeout: float = 2, + read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, @@ -745,16 +745,37 @@ def run_polling( * > 0 - retry up to X times read_timeout (:obj:`float`, optional): Value to pass to - :paramref:`telegram.Bot.get_updates.read_timeout`. Defaults to ``2``. + :paramref:`telegram.Bot.get_updates.read_timeout`. Defaults to + :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. versionchanged:: NEXT.VERSION + Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE` instead of + ``2``. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_read_timeout`. write_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to :paramref:`telegram.Bot.get_updates.write_timeout`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_write_timeout`. connect_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to :paramref:`telegram.Bot.get_updates.connect_timeout`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_connect_timeout`. pool_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to :paramref:`telegram.Bot.get_updates.pool_timeout`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_pool_timeout`. drop_pending_updates (:obj:`bool`, optional): Whether to clean any pending updates on Telegram servers before actually starting to poll. Default is :obj:`False`. allowed_updates (List[:obj:`str`], optional): Passed to @@ -783,6 +804,14 @@ def run_polling( "Application.run_polling is only available if the application has an Updater." ) + if (read_timeout, write_timeout, connect_timeout, pool_timeout) != ((DEFAULT_NONE,) * 4): + warn( + "Setting timeouts via `Application.run_polling` is deprecated. " + "Please use `ApplicationBuilder.get_updates_*_timeout` instead.", + PTBDeprecationWarning, + stacklevel=2, + ) + def error_callback(exc: TelegramError) -> None: self.create_task(self.process_error(error=exc, update=None)) diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index d5e094febfa..87ea704aed5 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -549,7 +549,7 @@ async def get_updates( timeout: Optional[int] = None, allowed_updates: Optional[Sequence[str]] = None, *, - read_timeout: float = 2, + read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, diff --git a/telegram/ext/_updater.py b/telegram/ext/_updater.py index 01d3ec524be..5949b1e01ba 100644 --- a/telegram/ext/_updater.py +++ b/telegram/ext/_updater.py @@ -211,7 +211,7 @@ async def start_polling( poll_interval: float = 0.0, timeout: int = 10, bootstrap_retries: int = -1, - read_timeout: float = 2, + read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, @@ -236,16 +236,40 @@ async def start_polling( * 0 - no retries * > 0 - retry up to X times read_timeout (:obj:`float`, optional): Value to pass to - :paramref:`telegram.Bot.get_updates.read_timeout`. Defaults to ``2``. + :paramref:`telegram.Bot.get_updates.read_timeout`. Defaults to + :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. versionchanged:: NEXT.VERSION + Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE` instead of + ``2``. + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_read_timeout` or + :paramref:`telegram.Bot.get_updates_request`. write_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to :paramref:`telegram.Bot.get_updates.write_timeout`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_write_timeout` or + :paramref:`telegram.Bot.get_updates_request`. connect_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to :paramref:`telegram.Bot.get_updates.connect_timeout`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_connect_timeout` or + :paramref:`telegram.Bot.get_updates_request`. pool_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to :paramref:`telegram.Bot.get_updates.pool_timeout`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + + .. deprecated:: NEXT.VERSION + Deprecated in favor of setting the timeout via + :meth:`telegram.ext.ApplicationBuilder.get_updates_pool_timeout` or + :paramref:`telegram.Bot.get_updates_request`. allowed_updates (List[:obj:`str`], optional): Passed to :meth:`telegram.Bot.get_updates`. drop_pending_updates (:obj:`bool`, optional): Whether to clean any pending updates on @@ -271,6 +295,10 @@ def callback(error: telegram.error.TelegramError) :exc:`RuntimeError`: If the updater is already running or was not initialized. """ + # We refrain from issuing deprecation warnings for the timeout parameters here, as we + # already issue them in `Application`. This means that there are no warnings when using + # `Updater` without `Application`, but this is a rather special use case. + if error_callback and asyncio.iscoroutinefunction(error_callback): raise TypeError( "The `error_callback` must not be a coroutine function! Use an ordinary function " @@ -316,7 +344,7 @@ async def _start_polling( self, poll_interval: float, timeout: int, - read_timeout: float, + read_timeout: ODVInput[float], write_timeout: ODVInput[float], connect_timeout: ODVInput[float], pool_timeout: ODVInput[float], @@ -401,16 +429,25 @@ async def _get_updates_cleanup() -> None: _LOGGER.debug( "Calling `get_updates` one more time to mark all fetched updates as read." ) - await self.bot.get_updates( - offset=self._last_update_id, - # We don't want to do long polling here! - timeout=0, - read_timeout=read_timeout, - connect_timeout=connect_timeout, - write_timeout=write_timeout, - pool_timeout=pool_timeout, - allowed_updates=allowed_updates, - ) + try: + await self.bot.get_updates( + offset=self._last_update_id, + # We don't want to do long polling here! + timeout=0, + read_timeout=read_timeout, + connect_timeout=connect_timeout, + write_timeout=write_timeout, + pool_timeout=pool_timeout, + allowed_updates=allowed_updates, + ) + except TelegramError as exc: + _LOGGER.error( + "Error while calling `get_updates` one more time to mark all fetched updates " + "as read: %s. Suppressing error to ensure graceful shutdown. When polling for " + "updates is restarted, updates may be fetched again. Please adjust timeouts " + "via `ApplicationBuilder` or the parameter `get_updates_request` of `Bot`.", + exc_info=exc, + ) self.__polling_cleanup_cb = _get_updates_cleanup diff --git a/telegram/request/_baserequest.py b/telegram/request/_baserequest.py index 2ae2a751998..58b0b6b7b24 100644 --- a/telegram/request/_baserequest.py +++ b/telegram/request/_baserequest.py @@ -130,6 +130,24 @@ async def __aexit__( # https://docs.python.org/3/reference/datamodel.html?#object.__aexit__ await self.shutdown() + @property + def read_timeout(self) -> Optional[float]: + """This property must return the default read timeout in seconds used by this class. + More precisely, the returned value should be the one used when + :paramref:`post.read_timeout` of :meth:post` is not passed/equal to :attr:`DEFAULT_NONE`. + + .. versionadded:: NEXT.VERSION + + Warning: + For now this property does not need to be implemented by subclasses and will raise + :exc:`NotImplementedError` if accessed without being overridden. However, in future + versions, this property will be abstract and must be implemented by subclasses. + + Returns: + :obj:`float` | :obj:`None`: The read timeout in seconds. + """ + raise NotImplementedError + @abc.abstractmethod async def initialize(self) -> None: """Initialize resources used by this class. Must be implemented by a subclass.""" diff --git a/telegram/request/_httpxrequest.py b/telegram/request/_httpxrequest.py index f1e298624a8..7d8c99500e7 100644 --- a/telegram/request/_httpxrequest.py +++ b/telegram/request/_httpxrequest.py @@ -198,6 +198,16 @@ def http_version(self) -> str: """ return self._http_version + @property + def read_timeout(self) -> Optional[float]: + """See :attr:`BaseRequest.read_timeout`. + + Returns: + :obj:`float` | :obj:`None`: The default read timeout in seconds as passed to + :paramref:`HTTPXRequest.read_timeout`. + """ + return self._client.timeout.read + def _build_client(self) -> httpx.AsyncClient: return httpx.AsyncClient(**self._client_kwargs) # type: ignore[arg-type] diff --git a/tests/ext/test_application.py b/tests/ext/test_application.py index cdd1ba73cc3..a51ea46e2a9 100644 --- a/tests/ext/test_application.py +++ b/tests/ext/test_application.py @@ -1498,6 +1498,54 @@ def thread_target(): found_log = True assert found_log + @pytest.mark.parametrize( + "timeout_name", + ["read_timeout", "connect_timeout", "write_timeout", "pool_timeout", "poll_interval"], + ) + @pytest.mark.skipif( + platform.system() == "Windows", + reason="Can't send signals without stopping whole process on windows", + ) + def test_run_polling_timeout_deprecation_warnings( + self, timeout_name, monkeypatch, recwarn, app + ): + async def get_updates(*args, **kwargs): + # This makes sure that other coroutines have a chance of running as well + await asyncio.sleep(0) + return [] + + def thread_target(): + waited = 0 + while not app.running: + time.sleep(0.05) + waited += 0.05 + if waited > 5: + pytest.fail("App apparently won't start") + + time.sleep(0.05) + + os.kill(os.getpid(), signal.SIGINT) + + monkeypatch.setattr(app.bot, "get_updates", get_updates) + + thread = Thread(target=thread_target) + thread.start() + + kwargs = {timeout_name: 42} + app.run_polling(drop_pending_updates=True, close_loop=False, **kwargs) + thread.join() + + if timeout_name == "poll_interval": + assert len(recwarn) == 0 + return + + assert len(recwarn) == 1 + assert "Setting timeouts via `Application.run_polling` is deprecated." in str( + recwarn[0].message + ) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__, "wrong stacklevel" + @pytest.mark.skipif( platform.system() == "Windows", reason="Can't send signals without stopping whole process on windows", @@ -2210,7 +2258,8 @@ async def raise_method(*args, **kwargs): else: app.run_webhook(close_loop=False, stop_signals=None) - assert len(recwarn) == 0 + for record in recwarn: + assert not str(record.message).startswith("Could not add signal handlers for the stop") @pytest.mark.flaky(3, 1) # loop.call_later will error the test when a flood error is received def test_signal_handlers(self, app, monkeypatch): diff --git a/tests/ext/test_applicationbuilder.py b/tests/ext/test_applicationbuilder.py index 15e328d140e..1655f94572f 100644 --- a/tests/ext/test_applicationbuilder.py +++ b/tests/ext/test_applicationbuilder.py @@ -19,11 +19,13 @@ import asyncio import inspect from dataclasses import dataclass +from http import HTTPStatus import httpx import pytest from telegram import Bot +from telegram._utils.defaultvalue import DEFAULT_NONE from telegram.ext import ( AIORateLimiter, Application, @@ -581,3 +583,32 @@ def test_get_updates_proxy_url_deprecation_warning(self, bot, builder, recwarn): ) assert recwarn[0].category is PTBDeprecationWarning assert recwarn[0].filename == __file__, "wrong stacklevel" + + @pytest.mark.parametrize( + ("read_timeout", "timeout", "expected"), + [ + (None, None, 0), + (1, None, 1), + (None, 1, 1), + (DEFAULT_NONE, None, 10), + (DEFAULT_NONE, 1, 11), + (1, 2, 3), + ], + ) + async def test_get_updates_read_timeout_value_passing( + self, bot, read_timeout, timeout, expected, monkeypatch, builder + ): + # This test is a double check that ApplicationBuilder respects the changes of #3963 just + # like `Bot` does - see also the corresponding test in test_bot.py (same name) + caught_read_timeout = None + + async def catch_timeouts(*args, **kwargs): + nonlocal caught_read_timeout + caught_read_timeout = kwargs.get("read_timeout") + return HTTPStatus.OK, b'{"ok": "True", "result": {}}' + + monkeypatch.setattr(HTTPXRequest, "do_request", catch_timeouts) + + bot = builder.get_updates_read_timeout(10).token(bot.token).build().bot + await bot.get_updates(read_timeout=read_timeout, timeout=timeout) + assert caught_read_timeout == expected diff --git a/tests/ext/test_updater.py b/tests/ext/test_updater.py index eeb86a5649e..f346b48bb9b 100644 --- a/tests/ext/test_updater.py +++ b/tests/ext/test_updater.py @@ -331,6 +331,39 @@ async def get_updates(*args, **kwargs): assert log_found + async def test_polling_mark_updates_as_read_timeout(self, monkeypatch, updater, caplog): + timeout_event = asyncio.Event() + + async def get_updates(*args, **kwargs): + await asyncio.sleep(0) + if timeout_event.is_set(): + raise TimedOut("TestMessage") + return [] + + monkeypatch.setattr(updater.bot, "get_updates", get_updates) + + async with updater: + await updater.start_polling() + with caplog.at_level(logging.ERROR): + timeout_event.set() + await updater.stop() + + assert len(caplog.records) >= 1 + log_found = False + for record in caplog.records: + if not record.getMessage().startswith( + "Error while calling `get_updates` one more time" + ): + continue + + assert record.name == "telegram.ext.Updater" + assert record.exc_info[0] is TimedOut + assert str(record.exc_info[1]) == "TestMessage" + log_found = True + break + + assert log_found + async def test_polling_mark_updates_as_read_failure(self, monkeypatch, updater, caplog): async def get_updates(*args, **kwargs): await asyncio.sleep(0) @@ -376,7 +409,7 @@ async def test_start_polling_get_updates_parameters(self, updater, monkeypatch): expected = { "timeout": 10, - "read_timeout": 2, + "read_timeout": DEFAULT_NONE, "write_timeout": DEFAULT_NONE, "connect_timeout": DEFAULT_NONE, "pool_timeout": DEFAULT_NONE, diff --git a/tests/request/test_request.py b/tests/request/test_request.py index ea95870705e..23143779d0d 100644 --- a/tests/request/test_request.py +++ b/tests/request/test_request.py @@ -353,6 +353,20 @@ async def make_assertion(*args, **kwargs): ) assert self.test_flag == (1, 2, 3, 4) + def test_read_timeout_not_implemented(self): + class SimpleRequest(BaseRequest): + async def do_request(self, *args, **kwargs): + raise httpx.ReadTimeout("read timeout") + + async def initialize(self) -> None: + pass + + async def shutdown(self) -> None: + pass + + with pytest.raises(NotImplementedError): + SimpleRequest().read_timeout + @pytest.mark.parametrize("media", [True, False]) async def test_timeout_propagation_write_timeout( self, monkeypatch, media, input_media_photo, recwarn # noqa: F811 @@ -739,6 +753,10 @@ def init_transport(*args, **kwargs): HTTPXRequest(socket_options=((1, 2, 3),)) assert transport_kwargs["socket_options"] == ((1, 2, 3),) + @pytest.mark.parametrize("read_timeout", [None, 1, 2, 3]) + async def test_read_timeout_property(self, read_timeout): + assert HTTPXRequest(read_timeout=read_timeout).read_timeout == read_timeout + @pytest.mark.skipif(not TEST_WITH_OPT_DEPS, reason="No need to run this twice") class TestHTTPXRequestWithRequest: diff --git a/tests/test_bot.py b/tests/test_bot.py index ea90cdd3414..d6277cf0e40 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -26,6 +26,7 @@ import socket import time from collections import defaultdict +from http import HTTPStatus import httpx import pytest @@ -79,7 +80,7 @@ from telegram.ext import ExtBot, InvalidCallbackData from telegram.helpers import escape_markdown from telegram.request import BaseRequest, HTTPXRequest, RequestData -from telegram.warnings import PTBUserWarning +from telegram.warnings import PTBDeprecationWarning, PTBUserWarning from tests.auxil.bot_method_checks import check_defaults_handling from tests.auxil.ci_bots import FALLBACKS from tests.auxil.envvars import GITHUB_ACTION, TEST_WITH_OPT_DEPS @@ -2447,6 +2448,63 @@ async def test_get_updates(self, bot): if updates: assert isinstance(updates[0], Update) + @pytest.mark.parametrize("bot_class", [Bot, ExtBot]) + async def test_get_updates_read_timeout_deprecation_warning( + self, bot, recwarn, monkeypatch, bot_class + ): + # Using the normal HTTPXRequest should not issue any warnings + await bot.get_updates() + assert len(recwarn) == 0 + + # Now let's test deprecation warning when using get_updates for other BaseRequest + # subclasses (we just monkeypatch the existing HTTPXRequest for this) + read_timeout = None + + async def catch_timeouts(*args, **kwargs): + nonlocal read_timeout + read_timeout = kwargs.get("read_timeout") + return HTTPStatus.OK, b'{"ok": "True", "result": {}}' + + monkeypatch.setattr(HTTPXRequest, "read_timeout", BaseRequest.read_timeout) + monkeypatch.setattr(HTTPXRequest, "do_request", catch_timeouts) + + bot = bot_class(get_updates_request=HTTPXRequest(), token=bot.token) + await bot.get_updates() + + assert len(recwarn) == 1 + assert "does not override the property `read_timeout`" in str(recwarn[0].message) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__, "wrong stacklevel" + + assert read_timeout == 2 + + @pytest.mark.parametrize( + ("read_timeout", "timeout", "expected"), + [ + (None, None, 0), + (1, None, 1), + (None, 1, 1), + (DEFAULT_NONE, None, 10), + (DEFAULT_NONE, 1, 11), + (1, 2, 3), + ], + ) + async def test_get_updates_read_timeout_value_passing( + self, bot, read_timeout, timeout, expected, monkeypatch + ): + caught_read_timeout = None + + async def catch_timeouts(*args, **kwargs): + nonlocal caught_read_timeout + caught_read_timeout = kwargs.get("read_timeout") + return HTTPStatus.OK, b'{"ok": "True", "result": {}}' + + monkeypatch.setattr(HTTPXRequest, "do_request", catch_timeouts) + + bot = Bot(get_updates_request=HTTPXRequest(read_timeout=10), token=bot.token) + await bot.get_updates(read_timeout=read_timeout, timeout=timeout) + assert caught_read_timeout == expected + @pytest.mark.xdist_group("getUpdates_and_webhook") @pytest.mark.parametrize("use_ip", [True, False]) # local file path as file_input is tested below in test_set_webhook_params