diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f061c910..479a8fd42 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/icloudpd/download.py b/src/icloudpd/download.py index b89ae2067..2ebd4cbcf 100644 --- a/src/icloudpd/download.py +++ b/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: diff --git a/src/pyicloud_ipd/base.py b/src/pyicloud_ipd/base.py index e0240e8f7..824d6e86f 100644 --- a/src/pyicloud_ipd/base.py +++ b/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 ) diff --git a/src/pyicloud_ipd/session.py b/src/pyicloud_ipd/session.py index a5d89ace8..7996e5451 100644 --- a/src/pyicloud_ipd/session.py +++ b/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 diff --git a/tests/test_download_photos_id.py b/tests/test_download_photos_id.py index ff9b4f854..0d2cd7b4a 100644 --- a/tests/test_download_photos_id.py +++ b/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])