:Setting timeout for HTTP: requests does not hanging forever if the download speed is zero (addressing an issue when Apple does not response to HTTP GET)

Author: naszta

src/icloudpd/download.py

@@ -6,7 +6,7 @@
 import socket
 import time
 
-from requests import Response
+from requests import Response, Timeout
 from requests.exceptions import ConnectionError
 from tzlocal import get_localzone
 
@@ -125,7 +125,7 @@ def download_media(
             )
             break
 
-        except (ConnectionError, socket.timeout, PyiCloudAPIResponseException) as ex:
+        except (ConnectionError, socket.timeout, PyiCloudAPIResponseException, Timeout) as ex:
             if "Invalid global session" in str(ex):
                 logger.error("Session error, re-authenticating...")
                 if retries > 0:

src/pyicloud_ipd/session.py

@@ -35,7 +35,7 @@ def filter(self, record: logging.LogRecord) -> bool:
         message = record.getMessage()
         if self.name in message:
             record.msg = message.replace(self.name, "********")
-            record.args = [] # type: ignore[assignment] 
+            record.args = [] # type: ignore[assignment]
 
         return True
 
@@ -45,11 +45,12 @@ class PyiCloudSession(Session):
 
     def __init__(self, service: Any):
         self.service = service
+        self._timeout = 30.0
         super().__init__()
 
     @override
-    # type: ignore 
-    def request(self, method: str, url, **kwargs):  
+    # type: ignore
+    def request(self, method: str, url, **kwargs):
 
         # Charge logging to the right service endpoint
         callee = inspect.stack()[2]
@@ -62,6 +63,8 @@ def request(self, method: str, url, **kwargs):
 
         has_retried = kwargs.get("retried")
         kwargs.pop("retried", None)
+        if "timeout" not in kwargs and self._timeout is not None:
+            kwargs["timeout"] = self._timeout
         response = super().request(method, url, **kwargs)
 
         content_type = response.headers.get("Content-Type", "").split(";")[0]
@@ -131,7 +134,7 @@ def request(self, method: str, url, **kwargs):
 
         try:
             data = response.json() if response.status_code != 204 else {}
-        except:  
+        except:
             request_logger.warning("Failed to parse response with JSON mimetype")
             return response
 

tests/test_download_photos_id.py

@@ -13,7 +13,7 @@
 import pytest
 from click.testing import CliRunner
 from piexif._exceptions import InvalidImageDataError
-from requests import Response
+from requests import Response, Timeout
 from requests.exceptions import ConnectionError
 from vcr import VCR
 
@@ -543,11 +543,11 @@ def mocked_authenticate(self: PyiCloudService) -> None:
 
                     assert result.exit_code == 1
 
-    def test_handle_connection_error_name_id7(self) -> None:
+    def test_timeout_error(self) -> None:
         base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
 
         def mock_raise_response_error(_arg: Any) -> NoReturn:
-            raise ConnectionError("Connection Error")
+            raise Timeout("Connection Error")
 
         with mock.patch.object(PhotoAsset, "download") as pa_download:
             pa_download.side_effect = mock_raise_response_error
@@ -599,6 +599,63 @@ def mocked_authenticate(self: PyiCloudService) -> None:
                     )
                     assert result.exit_code == 0
 
+    def test_handle_connection_error_name_id7(self) -> None:
+            base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
+
+            def mock_raise_response_error(_arg: Any) -> NoReturn:
+                raise ConnectionError("Connection Error")
+
+            with mock.patch.object(PhotoAsset, "download") as pa_download:
+                pa_download.side_effect = mock_raise_response_error
+
+                # Let the initial authenticate() call succeed,
+                # but do nothing on the second try.
+                orig_authenticate = PyiCloudService.authenticate
+
+                def mocked_authenticate(self: PyiCloudService) -> None:
+                    if not hasattr(self, "already_authenticated"):
+                        orig_authenticate(self)
+                        setattr(self, "already_authenticated", True)  # noqa: B010
+
+                with mock.patch("icloudpd.constants.WAIT_SECONDS", 0):  # noqa: SIM117
+                    with mock.patch.object(PyiCloudService, "authenticate", new=mocked_authenticate):
+                        _, result = run_icloudpd_test(
+                            self.assertEqual,
+                            self.root_path,
+                            base_dir,
+                            "listing_photos.yml",
+                            [],
+                            [],
+                            [
+                                "--username",
+                                "jdoe@gmail.com",
+                                "--password",
+                                "password1",
+                                "--recent",
+                                "1",
+                                "--skip-videos",
+                                "--skip-live-photos",
+                                "--no-progress-bar",
+                                "--file-match-policy",
+                                "name-id7",
+                            ],
+                        )
+
+                        # Error msg should be repeated 5 times
+                        assert (
+                            self._caplog.text.count(
+                                "Error downloading IMG_7409_QVk2Yyt.JPG, retrying after 0 seconds..."
+                            )
+                            == 5
+                        )
+
+                        self.assertIn(
+                            "ERROR    Could not download IMG_7409_QVk2Yyt.JPG. Please try again later.",
+                            self._caplog.text,
+                        )
+                        assert result.exit_code == 0
+
+
     def test_handle_albums_error_name_id7(self) -> None:
         base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])