Adjust read_timeout Behavior for Bot.get_updates (#3963)

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:

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)
 

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,

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))
 

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,

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
 

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."""

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]