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) (#1114)

Author: naszta

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- fix: timeout set to 30 seconds for HTTP requests [#793](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/793)
+
 ## 1.27.4 (2025-04-15)
 
 - fix: broken pypi publishing [#1105](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/1105)
@@ -87,7 +89,7 @@
 
 ## 1.23.0 (2024-07-25)
 
-- feature: update webui and allow to cancel and resume sync 
+- feature: update webui and allow to cancel and resume sync
 - deprecate linux 386 and arm v6 support
 - add linux musl builds
 

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/base.py

@@ -65,24 +65,25 @@ class PyiCloudService:
     """
 
     def __init__(
-        self, 
+        self,
         filename_cleaner: Callable[[str], str],
         lp_filename_generator: Callable[[str], str],
-        domain:str, 
+        domain:str,
         raw_policy: RawTreatmentPolicy,
         file_match_policy: FileMatchPolicy,
         apple_id: str, password:str, cookie_directory:Optional[str]=None, verify:bool=True,
-        client_id:Optional[str]=None, with_family:bool=True,
+        client_id:Optional[str]=None, with_family:bool=True, http_timeout:float=30.0
     ):
         self.filename_cleaner = filename_cleaner
         self.lp_filename_generator = lp_filename_generator
         self.raw_policy = raw_policy
         self.file_match_policy = file_match_policy
         self.user: Dict[str, Any] = {"accountName": apple_id, "password": password}
-        self.data: Dict[str, Any] = {} 
+        self.data: Dict[str, Any] = {}
         self.params: Dict[str, Any] = {}
         self.client_id: str = client_id or ("auth-%s" % str(uuid1()).lower())
         self.with_family = with_family
+        self.http_timeout = http_timeout
 
         self.password_filter = PyiCloudPasswordFilter(password)
         LOGGER.addFilter(self.password_filter)
@@ -120,7 +121,7 @@ def __init__(
         try:
             with open(self.session_path, encoding="utf-8") as session_f:
                 self.session_data = json.load(session_f)
-        except:  
+        except:
             LOGGER.info("Session file does not exist")
         session_client_id: Optional[str] = self.session_data.get("client_id")
         if session_client_id:
@@ -466,7 +467,7 @@ def get_trusted_phone_numbers(self) -> Sequence[TrustedDevice]:
         ).prepare()
 
         response = self.send_request(request)
-        
+
         return parse_trusted_phone_numbers_response(response)
 
     def send_2fa_code_sms(self, device_id: int) -> bool:
@@ -485,7 +486,7 @@ def send_2fa_code_sms(self, device_id: int) -> bool:
         ).prepare()
 
         response = self.send_request(request)
-        
+
         return response.ok
 
     def send_verification_code(self, device: Dict[str, Any]) -> bool:
@@ -537,7 +538,7 @@ def validate_2fa_code_sms(self, device_id: int, code:str) -> bool:
             json = req.json,
         ).prepare()
         response = self.send_request(request)
-        
+
         if response.ok:
             return self.trust_session()
         return False
@@ -591,7 +592,7 @@ def _get_webservice_url(self, ws_key: str) -> str:
         return typing.cast(str, self._webservices[ws_key]["url"])
 
     @property
-    def devices(self) -> Sequence[AppleDevice]: 
+    def devices(self) -> Sequence[AppleDevice]:
         """ Return all devices."""
         service_root = self._get_webservice_url("findme")
         return typing.cast(Sequence[AppleDevice], FindMyiPhoneServiceManager(
@@ -630,11 +631,11 @@ def photos(self) -> PhotosService:
         if not self._photos:
             service_root = self._get_webservice_url("ckdatabasews")
             self._photos = PhotosService(
-                service_root, 
-                self.session, 
-                self.params, 
-                self.filename_cleaner, 
-                self.lp_filename_generator, 
+                service_root,
+                self.session,
+                self.params,
+                self.filename_cleaner,
+                self.lp_filename_generator,
                 self.raw_policy,
                 self.file_match_policy
                 )

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
 
@@ -48,8 +48,8 @@ def __init__(self, service: Any):
         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 +62,8 @@ def request(self, method: str, url, **kwargs):
 
         has_retried = kwargs.get("retried")
         kwargs.pop("retried", None)
+        if "timeout" not in kwargs and self.service.http_timeout is not None:
+            kwargs["timeout"] = self.service.http_timeout
         response = super().request(method, url, **kwargs)
 
         content_type = response.headers.get("Content-Type", "").split(";")[0]
@@ -131,7 +133,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])