do not throw on connection errors #1187

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- fix: connection errors reported with stack trace [#1187](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/1187)
 - feat: `--skip-created-after` to limit assets by creation date [#466](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/466) [#1111](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/1111)
 - fix: connecting to a non-activated iCloud service reported as an error
 - fix: 503 response reported as an error [#1188](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/1188)

src/icloudpd/base.py

@@ -3,6 +3,9 @@
 
 from multiprocessing import freeze_support
 
+from requests import Timeout
+from requests.exceptions import ConnectionError
+
 import foundation
 from foundation.core import compose, constant, identity
 from icloudpd.mfa_provider import MFAProvider
@@ -1517,6 +1520,14 @@ def should_break(counter: Counter) -> bool:
                 return 1
             else:
                 pass
+        except (ConnectionError, TimeoutError, Timeout) as _error:
+            logger.info("Cannot connect to Apple iCloud service")
+            # logger.debug(error)
+            # it not watching then return error
+            if not watch_interval:
+                return 1
+            else:
+                pass
         except TwoStepAuthRequiredError:
             if notification_script is not None:
                 subprocess.call([notification_script])
@@ -1537,7 +1548,6 @@ def should_break(counter: Counter) -> bool:
             else:
                 pass
             return 1
-
         if watch_interval:  # pragma: no cover
             logger.info(f"Waiting for {watch_interval} sec...")
             interval: Sequence[int] = range(1, watch_interval)

src/icloudpd/download.py

@@ -5,8 +5,7 @@
 import os
 import time
 
-from requests import Response, Timeout
-from requests.exceptions import ConnectionError
+from requests import Response
 from tzlocal import get_localzone
 
 # Import the constants object so that we can mock WAIT_SECONDS in tests
@@ -124,7 +123,7 @@ def download_media(
             )
             break
 
-        except (TimeoutError, ConnectionError, PyiCloudAPIResponseException, Timeout) as ex:
+        except PyiCloudAPIResponseException as ex:
             if "Invalid global session" in str(ex):
                 logger.error("Session error, re-authenticating...")
                 if retries > 0:

tests/test_authentication.py

@@ -1,11 +1,13 @@
 import inspect
 import os
 import shutil
-from typing import NamedTuple
-from unittest import TestCase
+from typing import Any, NamedTuple, NoReturn
+from unittest import TestCase, mock
 
 import pytest
 from click.testing import CliRunner
+from requests import Timeout
+from requests.exceptions import ConnectionError
 from vcr import VCR
 
 # import vcr
@@ -18,6 +20,7 @@
 from icloudpd.status import StatusExchange
 from pyicloud_ipd.file_match import FileMatchPolicy
 from pyicloud_ipd.raw_policy import RawTreatmentPolicy
+from pyicloud_ipd.session import PyiCloudSession
 from pyicloud_ipd.sms import parse_trusted_phone_numbers_payload
 from tests.helpers import path_from_project_root, recreate_path
 
@@ -413,6 +416,141 @@ def test_failed_auth_503_watch(self) -> None:
             # self.assertTrue("Can't overwrite existing cassette" in str(context.exception))
             assert result.exit_code == 1  # should error for vcr
 
+    def test_connection_error(self) -> None:
+        base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
+        cookie_dir = os.path.join(base_dir, "cookie")
+
+        for dir in [base_dir, cookie_dir]:
+            recreate_path(dir)
+
+        def mock_raise_response_error(_a1: Any, _a2: Any, _a3: Any, **kwargs) -> NoReturn:  # type: ignore [no-untyped-def]
+            raise ConnectionError("Simulated Connection Error")
+
+        with (
+            mock.patch.object(
+                PyiCloudSession, "request", side_effect=mock_raise_response_error, autospec=True
+            ) as pa_request,
+            vcr.use_cassette(os.path.join(self.vcr_path, "failed_auth_503.yml")),
+        ):  # noqa: SIM117
+            # errors.CannotOverwriteExistingCassetteException
+            runner = CliRunner(env={"CLIENT_ID": "EC5646DE-9423-11E8-BF21-14109FE0B321"})
+            result = runner.invoke(
+                main,
+                [
+                    "--username",
+                    "jdoe@gmail.com",
+                    "--password",
+                    "password1",
+                    "--no-progress-bar",
+                    "--directory",
+                    base_dir,
+                    "--cookie-directory",
+                    cookie_dir,
+                    # "--watch-with-interval",
+                    # "1",
+                ],
+            )
+            pa_request.assert_called_once()
+            self.assertIn(
+                "Authenticating...",
+                self._caplog.text,
+            )
+            self.assertIn(
+                "INFO     Cannot connect to Apple iCloud service",
+                self._caplog.text,
+            )
+            assert result.exit_code == 1  # should error for vcr
+
+    def test_timeout_error(self) -> None:
+        base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
+        cookie_dir = os.path.join(base_dir, "cookie")
+
+        for dir in [base_dir, cookie_dir]:
+            recreate_path(dir)
+
+        def mock_raise_response_error(_a1: Any, _a2: Any, _a3: Any, **kwargs) -> NoReturn:  # type: ignore [no-untyped-def]
+            raise TimeoutError("Simulated TimeoutError")
+
+        with (
+            mock.patch.object(
+                PyiCloudSession, "request", side_effect=mock_raise_response_error, autospec=True
+            ) as pa_request,
+            vcr.use_cassette(os.path.join(self.vcr_path, "failed_auth_503.yml")),
+        ):  # noqa: SIM117
+            # errors.CannotOverwriteExistingCassetteException
+            runner = CliRunner(env={"CLIENT_ID": "EC5646DE-9423-11E8-BF21-14109FE0B321"})
+            result = runner.invoke(
+                main,
+                [
+                    "--username",
+                    "jdoe@gmail.com",
+                    "--password",
+                    "password1",
+                    "--no-progress-bar",
+                    "--directory",
+                    base_dir,
+                    "--cookie-directory",
+                    cookie_dir,
+                    # "--watch-with-interval",
+                    # "1",
+                ],
+            )
+            pa_request.assert_called_once()
+            self.assertIn(
+                "Authenticating...",
+                self._caplog.text,
+            )
+            self.assertIn(
+                "INFO     Cannot connect to Apple iCloud service",
+                self._caplog.text,
+            )
+            assert result.exit_code == 1  # should error for vcr
+
+    def test_timeout(self) -> None:
+        base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])
+        cookie_dir = os.path.join(base_dir, "cookie")
+
+        for dir in [base_dir, cookie_dir]:
+            recreate_path(dir)
+
+        def mock_raise_response_error(_a1: Any, _a2: Any, _a3: Any, **kwargs) -> NoReturn:  # type: ignore [no-untyped-def]
+            raise Timeout("Simulated Timeout")
+
+        with (
+            mock.patch.object(
+                PyiCloudSession, "request", side_effect=mock_raise_response_error, autospec=True
+            ) as pa_request,
+            vcr.use_cassette(os.path.join(self.vcr_path, "failed_auth_503.yml")),
+        ):  # noqa: SIM117
+            # errors.CannotOverwriteExistingCassetteException
+            runner = CliRunner(env={"CLIENT_ID": "EC5646DE-9423-11E8-BF21-14109FE0B321"})
+            result = runner.invoke(
+                main,
+                [
+                    "--username",
+                    "jdoe@gmail.com",
+                    "--password",
+                    "password1",
+                    "--no-progress-bar",
+                    "--directory",
+                    base_dir,
+                    "--cookie-directory",
+                    cookie_dir,
+                    # "--watch-with-interval",
+                    # "1",
+                ],
+            )
+            pa_request.assert_called_once()
+            self.assertIn(
+                "Authenticating...",
+                self._caplog.text,
+            )
+            self.assertIn(
+                "INFO     Cannot connect to Apple iCloud service",
+                self._caplog.text,
+            )
+            assert result.exit_code == 1  # should error for vcr
+
 
 class _TrustedDevice(NamedTuple):
     id: int

tests/test_download_photos.py

@@ -15,7 +15,6 @@
 from click.testing import CliRunner
 from piexif._exceptions import InvalidImageDataError
 from requests import Response
-from requests.exceptions import ConnectionError
 from vcr import VCR
 
 from icloudpd import constants
@@ -543,62 +542,6 @@ def mocked_authenticate(self: PyiCloudService) -> None:
 
                     assert result.exit_code == 1
 
-    def test_handle_connection_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")
-
-        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",
-                            "--threads-num",
-                            "1",
-                        ],
-                    )
-
-                    # Error msg should be repeated 5 times
-                    assert (
-                        self._caplog.text.count(
-                            "Error downloading IMG_7409.JPG, retrying after 0 seconds..."
-                        )
-                        == 5
-                    )
-
-                    self.assertIn(
-                        "ERROR    Could not download IMG_7409.JPG. Please try again later.",
-                        self._caplog.text,
-                    )
-                    assert result.exit_code == 0
-
     def test_handle_albums_error(self) -> None:
         base_dir = os.path.join(self.fixtures_path, inspect.stack()[0][3])