Improve Exception Handling in File.download_* (#4542)

telegram/_files/file.py

@@ -128,9 +128,8 @@ class File(TelegramObject):
     ) -> Path:
         """
         Download this file. By default, the file is saved in the current working directory with
-        :attr:`file_path` as file name. If the file has no filename, the file ID will be used as
-        filename. If :paramref:`custom_path` is supplied as a :obj:`str` or :obj:`pathlib.Path`,
-        it will be saved to that path.
+        :attr:`file_path` as file name. If :paramref:`custom_path` is supplied as a :obj:`str` or
+        :obj:`pathlib.Path`, it will be saved to that path.
 
         Note:
             If :paramref:`custom_path` isn't provided and :attr:`file_path` is the path of a
@@ -152,6 +151,11 @@ class File(TelegramObject):
             * This method was previously called ``download``. It was split into
               :meth:`download_to_drive` and :meth:`download_to_memory`.
 
+        .. versionchanged:: NEXT.VERSION
+            Raises :exc:`RuntimeError` if :attr:`file_path` is not set. Note that files without
+            a :attr:`file_path` could never be downloaded, as this attribute is mandatory for that
+            operation.
+
         Args:
             custom_path (:class:`pathlib.Path` | :obj:`str` , optional): The path where the file
                 will be saved to. If not specified, will be saved in the current working directory
@@ -175,7 +179,13 @@ class File(TelegramObject):
         Returns:
             :class:`pathlib.Path`: Returns the Path object the file was downloaded to.
 
+        Raises:
+            RuntimeError: If :attr:`file_path` is not set.
+
         """
+        if not self.file_path:
+            raise RuntimeError("No `file_path` available for this file. Can not download.")
+
         local_file = is_local_file(self.file_path)
         url = None if local_file else self._get_encoded_url()
 
@@ -198,10 +208,8 @@ class File(TelegramObject):
             filename = Path(custom_path)
         elif local_file:
             return Path(self.file_path)
-        elif self.file_path:
-            filename = Path(Path(self.file_path).name)
         else:
-            filename = Path.cwd() / self.file_id
+            filename = Path(Path(self.file_path).name)
 
         buf = await self.get_bot().request.retrieve(
             url,
@@ -237,6 +245,11 @@ class File(TelegramObject):
 
         .. versionadded:: 20.0
 
+        .. versionchanged:: NEXT.VERSION
+            Raises :exc:`RuntimeError` if :attr:`file_path` is not set. Note that files without
+            a :attr:`file_path` could never be downloaded, as this attribute is mandatory for that
+            operation.
+
         Args:
             out (:obj:`io.BufferedIOBase`): A file-like object. Must be opened for writing in
                 binary mode.
@@ -254,7 +267,13 @@ class File(TelegramObject):
             pool_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to
                 :paramref:`telegram.request.BaseRequest.post.pool_timeout`. Defaults to
                 :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.
+
+        Raises:
+            RuntimeError: If :attr:`file_path` is not set.
         """
+        if not self.file_path:
+            raise RuntimeError("No `file_path` available for this file. Can not download.")
+
         local_file = is_local_file(self.file_path)
         url = None if local_file else self._get_encoded_url()
         path = Path(self.file_path) if local_file else None
@@ -283,6 +302,11 @@ class File(TelegramObject):
     ) -> bytearray:
         """Download this file and return it as a bytearray.
 
+        .. versionchanged:: NEXT.VERSION
+            Raises :exc:`RuntimeError` if :attr:`file_path` is not set. Note that files without
+            a :attr:`file_path` could never be downloaded, as this attribute is mandatory for that
+            operation.
+
         Args:
             buf (:obj:`bytearray`, optional): Extend the given bytearray with the downloaded data.
 
@@ -312,7 +336,13 @@ class File(TelegramObject):
             :obj:`bytearray`: The same object as :paramref:`buf` if it was specified. Otherwise a
             newly allocated :obj:`bytearray`.
 
+        Raises:
+            RuntimeError: If :attr:`file_path` is not set.
+
         """
+        if not self.file_path:
+            raise RuntimeError("No `file_path` available for this file. Can not download.")
+
         if buf is None:
             buf = bytearray()
 

tests/_files/test_file.py

@@ -17,6 +17,7 @@
 # You should have received a copy of the GNU Lesser Public License
 # along with this program.  If not, see [http://www.gnu.org/licenses/].
 import os
+from io import BytesIO
 from pathlib import Path
 from tempfile import TemporaryFile, mkstemp
 
@@ -181,21 +182,6 @@ class TestFileWithoutRequest(FileTestBase):
             os.close(file_handle)
             custom_path.unlink(missing_ok=True)
 
-    async def test_download_no_filename(self, monkeypatch, file):
-        async def test(*args, **kwargs):
-            return self.file_content
-
-        file.file_path = None
-
-        monkeypatch.setattr(file.get_bot().request, "retrieve", test)
-        out_file = await file.download_to_drive()
-
-        assert str(out_file)[-len(file.file_id) :] == file.file_id
-        try:
-            assert out_file.read_bytes() == self.file_content
-        finally:
-            out_file.unlink(missing_ok=True)
-
     async def test_download_file_obj(self, monkeypatch, file):
         async def test(*args, **kwargs):
             return self.file_content
@@ -272,6 +258,14 @@ class TestFileWithoutRequest(FileTestBase):
         assert buf2[len(buf) :] == buf
         assert buf2[: len(buf)] == buf
 
+    async def test_download_no_file_path(self):
+        with pytest.raises(RuntimeError, match="No `file_path` available"):
+            await File(self.file_id, self.file_unique_id).download_to_drive()
+        with pytest.raises(RuntimeError, match="No `file_path` available"):
+            await File(self.file_id, self.file_unique_id).download_to_memory(BytesIO())
+        with pytest.raises(RuntimeError, match="No `file_path` available"):
+            await File(self.file_id, self.file_unique_id).download_as_bytearray()
+
 
 class TestFileWithRequest(FileTestBase):
     async def test_error_get_empty_file_id(self, bot):