remove retries to avoid rate limiting #1195

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- fix: retries trigger rate limiting [#1195](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/1195)
 - fix: smtp & script notifications terminate the program [#898](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/898)
 
 ## 1.29.0 (2025-07-19)

src/icloudpd/base.py

@@ -5,6 +5,7 @@
 
 from requests import Timeout
 from requests.exceptions import ConnectionError
+from urllib3.exceptions import NewConnectionError
 
 import foundation
 from foundation.core import compose, constant, identity
@@ -1229,42 +1230,34 @@ def retrier(
 
 
 def session_error_handle_builder(
-    logger: Logger, icloud: PyiCloudService
-) -> Callable[[Exception, int], None]:
-    """Build handler for session error"""
-
-    def session_error_handler(ex: Exception, attempt: int) -> None:
-        """Handles session errors in the PhotoAlbum photos iterator"""
-        if "Invalid global session" in str(ex):
-            if attempt > constants.MAX_RETRIES:
-                logger.error("iCloud re-authentication failed. Please try again later.")
-                raise ex
+    logger: Logger, icloud: PyiCloudService, ex: Exception, attempt: int
+) -> None:
+    """Handles session errors in the PhotoAlbum photos iterator"""
+    if "Invalid global session" in str(ex):
+        if constants.MAX_RETRIES == 0:
             logger.error("Session error, re-authenticating...")
-            if attempt > 1:
-                # If the first re-authentication attempt failed,
-                # start waiting a few seconds before retrying in case
-                # there are some issues with the Apple servers
-                time.sleep(constants.WAIT_SECONDS * attempt)
-            icloud.authenticate()
-
-    return session_error_handler
-
-
-def internal_error_handle_builder(logger: logging.Logger) -> Callable[[Exception, int], None]:
-    """Build handler for internal error"""
-
-    def internal_error_handler(ex: Exception, attempt: int) -> None:
-        """Handles session errors in the PhotoAlbum photos iterator"""
-        if "INTERNAL_ERROR" in str(ex):
-            if attempt > constants.MAX_RETRIES:
-                logger.error("Internal Error at Apple.")
-                raise ex
-            logger.error("Internal Error at Apple, retrying...")
+        if attempt > constants.MAX_RETRIES:
+            logger.error("iCloud re-authentication failed. Please try again later.")
+            raise ex
+        logger.error("Session error, re-authenticating...")
+        if attempt > 1:
+            # If the first re-authentication attempt failed,
             # start waiting a few seconds before retrying in case
             # there are some issues with the Apple servers
             time.sleep(constants.WAIT_SECONDS * attempt)
+        icloud.authenticate()
+
 
-    return internal_error_handler
+def internal_error_handle_builder(logger: logging.Logger, ex: Exception, attempt: int) -> None:
+    """Handles session errors in the PhotoAlbum photos iterator"""
+    if "INTERNAL_ERROR" in str(ex):
+        if attempt > constants.MAX_RETRIES:
+            logger.error("Internal Error at Apple.")
+            raise ex
+        logger.error("Internal Error at Apple, retrying...")
+        # start waiting a few seconds before retrying in case
+        # there are some issues with the Apple servers
+        time.sleep(constants.WAIT_SECONDS * attempt)
 
 
 def compose_handlers(
@@ -1378,8 +1371,8 @@ def core(
                 album_phrase = f" from album {album}" if album else ""
                 logger.debug(f"Looking up all photos{videos_phrase}{album_phrase}...")
 
-                session_exception_handler = session_error_handle_builder(logger, icloud)
-                internal_error_handler = internal_error_handle_builder(logger)
+                session_exception_handler = partial(session_error_handle_builder, logger, icloud)
+                internal_error_handler = partial(internal_error_handle_builder, logger)
 
                 error_handler = compose_handlers(
                     [session_exception_handler, internal_error_handler]
@@ -1541,7 +1534,7 @@ def should_break(counter: Counter) -> bool:
                 return 1
             else:
                 pass
-        except (ConnectionError, TimeoutError, Timeout) as _error:
+        except (ConnectionError, TimeoutError, Timeout, NewConnectionError) as _error:
             logger.info("Cannot connect to Apple iCloud service")
             # logger.debug(error)
             # it not watching then return error

src/icloudpd/constants.py

@@ -3,5 +3,5 @@
 from typing import Final
 
 # For retrying connection after timeouts and errors
-MAX_RETRIES: Final[int] = 5
+MAX_RETRIES: Final[int] = 0
 WAIT_SECONDS: Final[int] = 5

src/icloudpd/download.py

@@ -112,7 +112,8 @@ def download_media(
     if not mkdirs_local(logger, download_path):
         return False
 
-    for retries in range(constants.MAX_RETRIES):
+    retries = 0
+    while True:
         try:
             photo_response = photo.download(version.url)
             if photo_response:
@@ -134,6 +135,9 @@ def download_media(
 
                 icloud.authenticate()
             else:
+                # short circuiting 0 retries
+                if retries == constants.MAX_RETRIES:
+                    break
                 # you end up here when p.e. throttling by Apple happens
                 wait_time = (retries + 1) * constants.WAIT_SECONDS
                 logger.error(
@@ -150,7 +154,10 @@ def download_media(
                 download_path,
             )
             break
-    else:
+        retries = retries + 1
+        if retries >= constants.MAX_RETRIES:
+            break
+    if retries >= constants.MAX_RETRIES:
         logger.error(
             "Could not download %s. Please try again later.",
             photo.filename,

src/pyicloud_ipd/exceptions.py

@@ -10,14 +10,12 @@ class PyiCloudException(Exception):
 #API
 class PyiCloudAPIResponseException(PyiCloudException):
     """iCloud response exception."""
-    def __init__(self, reason:str, code:Optional[str]=None, retry:bool=False):
+    def __init__(self, reason:str, code:Optional[str]=None):
         self.reason = reason
         self.code = code
         message = reason or ""
         if code:
             message += " (%s)" % code
-        if retry:
-            message += ". Retrying ..."
 
         super().__init__(message)
 

src/pyicloud_ipd/session.py

@@ -60,8 +60,6 @@ def request(self, method: str, url, **kwargs): # type: ignore
 
         request_logger.debug("%s %s %s", method, url, kwargs.get("data", ""))
 
-        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 = throw_on_503(super().request(method, url, **kwargs))
@@ -91,35 +89,6 @@ def request(self, method: str, url, **kwargs): # type: ignore
             content_type not in json_mimetypes
             or response.status_code in [421, 450, 500]
         ):
-            try:
-                # pylint: disable=protected-access
-                fmip_url = self.service._get_webservice_url("findme")
-                if (
-                    has_retried is None
-                    and response.status_code in [421, 450, 500]
-                    and fmip_url in url
-                ):
-                    # Handle re-authentication for Find My iPhone
-                    LOGGER.debug("Re-authenticating Find My iPhone service")
-                    try:
-                        # If 450, authentication requires a full sign in to the account
-                        service = None if response.status_code == 450 else "find"
-                        self.service.authenticate(True, service)
-
-                    except PyiCloudAPIResponseException:
-                        LOGGER.debug("Re-authentication failed")
-                    kwargs["retried"] = True
-                    return self.request(method, url, **kwargs)
-            except Exception:
-                pass
-
-            if has_retried is None and response.status_code in [421, 450, 500]:
-                api_error = PyiCloudAPIResponseException(
-                    response.reason, str(response.status_code), True
-                )
-                request_logger.debug(api_error)
-                kwargs["retried"] = True
-                return self.request(method, url, **kwargs)
 
             self._raise_error(str(response.status_code), response.reason)
 

tests/test_autodelete_photos.py

@@ -359,6 +359,7 @@ def test_autodelete_photos(self) -> None:
                 f"{file_name} not expected, but present"
             )
 
+    @pytest.mark.skipif(constants.MAX_RETRIES == 0, reason="Disabled when MAX_RETRIES set to 0")
     def test_retry_delete_after_download_session_error(self) -> None:
         base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
         cookie_dir = os.path.join(base_dir, "cookie")
@@ -434,16 +435,24 @@ def mocked_authenticate(self: PyiCloudService) -> None:
                             self._caplog.text,
                         )
 
-                        # Error msg should be repeated 5 times
+                        # Error msg should be repeated always 1 time
                         self.assertEqual(
                             self._caplog.text.count("Session error, re-authenticating..."),
                             1,
-                            "Re-auth message count",
+                            "retry count",
                         )
 
-                        self.assertEqual(pa_delete.call_count, 2, "delete call count")
-                        # Make sure we only call sleep 4 times (skip the first retry)
-                        self.assertEqual(sleep_mock.call_count, 0, "Sleep call count")
+                        self.assertEqual(
+                            pa_delete.call_count,
+                            1 + min(1, constants.MAX_RETRIES),
+                            "delete call count",
+                        )
+                        # Make sure we only call sleep 0 times (skip the first retry)
+                        self.assertEqual(
+                            sleep_mock.call_count,
+                            0,
+                            "sleep count",
+                        )
                         self.assertEqual(result.exit_code, 0, "Exit code")
 
         # check files
@@ -529,19 +538,21 @@ def mocked_authenticate(self: PyiCloudService) -> None:
                             self._caplog.text,
                         )
 
-                        # Error msg should be repeated 5 times
+                        # Error msg should be repeated MAX_RETRIES times
                         self.assertEqual(
                             self._caplog.text.count("Session error, re-authenticating..."),
-                            constants.MAX_RETRIES,
-                            "Re-auth message count",
+                            max(1, constants.MAX_RETRIES),
+                            "retry count",
                         )
 
                         self.assertEqual(
                             pa_delete.call_count, constants.MAX_RETRIES + 1, "delete call count"
                         )
-                        # Make sure we only call sleep 4 times (skip the first retry)
+                        # Make sure we only call sleep MAX_RETRIES-1 times (skip the first retry)
                         self.assertEqual(
-                            sleep_mock.call_count, constants.MAX_RETRIES - 1, "Sleep call count"
+                            sleep_mock.call_count,
+                            max(0, constants.MAX_RETRIES - 1),
+                            "sleep count",
                         )
                         self.assertEqual(result.exit_code, 1, "Exit code")
 
@@ -555,6 +566,7 @@ def mocked_authenticate(self: PyiCloudService) -> None:
 
         assert sum(1 for _ in files_in_result) == 1
 
+    @pytest.mark.skipif(constants.MAX_RETRIES == 0, reason="Disabled when MAX_RETRIES set to 0")
     def test_retry_delete_after_download_internal_error(self) -> None:
         base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
         cookie_dir = os.path.join(base_dir, "cookie")
@@ -616,16 +628,20 @@ def mock_raise_response_error(
                         self._caplog.text,
                     )
 
-                    # Error msg should be repeated 5 times
+                    # Error msg should be repeated MAX_RETRIES times
                     self.assertEqual(
                         self._caplog.text.count("Internal Error at Apple, retrying..."),
-                        1,
-                        "Retry message count",
+                        min(1, constants.MAX_RETRIES),
+                        "retry count",
                     )
 
-                    self.assertEqual(pa_delete.call_count, 2, "delete call count")
+                    self.assertEqual(
+                        pa_delete.call_count, 1 + min(1, constants.MAX_RETRIES), "delete count"
+                    )
                     # Make sure we only call sleep 4 times (skip the first retry)
-                    self.assertEqual(sleep_mock.call_count, 1, "Sleep call count")
+                    self.assertEqual(
+                        sleep_mock.call_count, min(1, constants.MAX_RETRIES), "sleep count"
+                    )
                     self.assertEqual(result.exit_code, 0, "Exit code")
 
         # check files
@@ -701,16 +717,14 @@ def mock_raise_response_error(
                     self.assertEqual(
                         self._caplog.text.count("Internal Error at Apple, retrying..."),
                         constants.MAX_RETRIES,
-                        "Retry message count",
+                        "retry count",
                     )
 
                     self.assertEqual(
-                        pa_delete.call_count, constants.MAX_RETRIES + 1, "delete call count"
+                        pa_delete.call_count, constants.MAX_RETRIES + 1, "delete count"
                     )
                     # Make sure we only call sleep N times (skip the first retry)
-                    self.assertEqual(
-                        sleep_mock.call_count, constants.MAX_RETRIES, "Sleep call count"
-                    )
+                    self.assertEqual(sleep_mock.call_count, constants.MAX_RETRIES, "sleep count")
                     self.assertEqual(result.exit_code, 1, "Exit code")
 
         # check files