Merge pull request ManiMatter#333 from tomerh2001/fix-detect-deletions-timeout-resilience

[NEEDS CODE REVIEWER] (time-out related) Fix detect_deletions disable gating and harden timeout handling

Author: jrhager84

README.md

@@ -20,6 +20,7 @@ Looking to **upgrade from V1 to V2**? Look [here](#upgrading-from-v1-to-v2)
     - [LOG_LEVEL](#log_level)
     - [TEST_RUN](#test_run)
     - [TIMER](#timer)
+    - [REQUEST_TIMEOUT](#request_timeout)
     - [SSL_VERIFICATION](#ssl_verification)
     - [IGNORE_DOWNLOAD_CLIENTS](#ignore_download_clients)
     - [PRIVATE_TRACKER_HANDLING / PUBLIC_TRACKER_HANDLING](#private_tracker_handling--public_tracker_handling)
@@ -193,6 +194,7 @@ services:
       LOG_LEVEL: INFO
       TEST_RUN: True
       TIMER: 10
+      # REQUEST_TIMEOUT: 15
       # IGNORED_DOWNLOAD_CLIENTS: >
       #   - emulerr
       # SSL_VERIFICATION: true
@@ -399,6 +401,13 @@ Configures the general behavior of the application (across all features)
 -   Unit: Minutes
 -   Is Mandatory: No (Defaults to 10)
 
+#### REQUEST_TIMEOUT
+
+-   Timeout used for HTTP/API requests to *arr and download clients
+-   Type: Integer or Float
+-   Unit: Seconds
+-   Is Mandatory: No (Defaults to 15)
+
 #### SSL_VERIFICATION
 
 -   Turns SSL certificate verification on or off for all API calls

config/config_example.yaml

@@ -2,6 +2,7 @@ general:
   log_level: INFO
   test_run: true
   timer: 10
+  # request_timeout: 15 # Optional: timeout for all HTTP/API calls in seconds
   # ignored_download_clients: ["emulerr"]
   # ssl_verification: false # Optional: Defaults to true
   # private_tracker_handling: "obsolete_tag" # remove, skip, obsolete_tag. Optional. Default: remove

main.py

@@ -52,23 +52,35 @@ async def wait_next_run():
 async def main():
     await launch_steps(settings)
 
-    if settings.jobs.detect_deletions:
+    if settings.jobs.detect_deletions.enabled:
         await WatcherManager(settings).setup()
     # Start Cleaning
     while True:
         logger.info("-" * 50)
 
         # Refresh qBit Cookies (SABnzbd doesn't need cookie refresh)
         for qbit in settings.download_clients.qbittorrent:
-            await qbit.refresh_cookie()
+            try:
+                await qbit.refresh_cookie()
+            except Exception as err:  # noqa: BLE001
+                logger.error(
+                    f"Error while refreshing cookie for {qbit.name} ({qbit.base_url}): {err}",
+                    exc_info=True,
+                )
 
         # Run script for each instance
         for arr in settings.instances:
             await job_manager.run_jobs(arr)
             logger.verbose("")
 
         # Run download client jobs (these run independently of *arr instances)
-        await job_manager.run_download_client_jobs()
+        try:
+            await job_manager.run_download_client_jobs()
+        except Exception as err:  # noqa: BLE001
+            logger.error(
+                f"Error while running download-client jobs: {err}",
+                exc_info=True,
+            )
 
         # Wait for the next run
         await wait_next_run()

src/job_manager.py

@@ -1,4 +1,6 @@
 # Cleans the download queue
+import requests
+
 from src.jobs.remove_bad_files import RemoveBadFiles
 from src.jobs.remove_done_seeding import RemoveDoneSeeding
 from src.jobs.remove_failed_downloads import RemoveFailedDownloads
@@ -24,12 +26,25 @@ def __init__(self, settings):
     async def run_jobs(self, arr):
         self.arr = arr
         logger.info(f"*** Running jobs on {self.arr.name} ({self.arr.base_url}) ***")
-        await self.removal_jobs()
-        await self.search_jobs()
+        await self._run_arr_job_group("Removal Jobs", self.removal_jobs)
+        await self._run_arr_job_group("Search Jobs", self.search_jobs)
 
     async def run_download_client_jobs(self):
         """Run jobs that operate on download clients directly."""
-        if not await self._download_clients_connected():
+        try:
+            if not await self._download_clients_connected():
+                return None
+        except requests.exceptions.RequestException as err:
+            logger.error(
+                f"Download client connectivity check failed with request error: {err}",
+                exc_info=True,
+            )
+            return None
+        except Exception as err:  # noqa: BLE001
+            logger.error(
+                f"Download client connectivity check failed: {err}",
+                exc_info=True,
+            )
             return None
 
         items_detected = 0
@@ -56,7 +71,9 @@ async def run_download_client_jobs(self):
 
                 for download_client_job in download_client_jobs:
                     if download_client_job.job.enabled:
-                        items_detected += await download_client_job.run()
+                        items_detected += await self._run_download_client_job(
+                            download_client_job, client
+                        )
 
         return items_detected
 
@@ -143,6 +160,37 @@ async def _check_client_connection_status(self, clients):
                 return False
         return True
 
+    async def _run_arr_job_group(self, label, job_runner):
+        """Run one ARR job group and keep the main loop alive on failures."""
+        try:
+            await job_runner()
+        except requests.exceptions.RequestException as err:
+            logger.error(
+                f"{label}: request error on {self.arr.name} ({self.arr.base_url}): {err}",
+                exc_info=True,
+            )
+        except Exception as err:  # noqa: BLE001
+            logger.error(
+                f"{label}: unexpected error on {self.arr.name} ({self.arr.base_url}): {err}",
+                exc_info=True,
+            )
+
+    async def _run_download_client_job(self, download_client_job, client):
+        """Run one download-client job and keep the loop alive on failures."""
+        try:
+            return await download_client_job.run()
+        except requests.exceptions.RequestException as err:
+            logger.error(
+                f"Download-client job '{download_client_job.job_name}' failed on {client.name} ({client.base_url}) due to request error: {err}",
+                exc_info=True,
+            )
+        except Exception as err:  # noqa: BLE001
+            logger.error(
+                f"Download-client job '{download_client_job.job_name}' failed on {client.name} ({client.base_url}): {err}",
+                exc_info=True,
+            )
+        return 0
+
     def _get_removal_jobs(self):
         """
         Return a list of enabled removal job instances based on the provided settings.

src/settings/_general.py

@@ -11,6 +11,7 @@ class General:
     log_level: str = "INFO"
     test_run: bool = False
     timer: float = 10.0
+    request_timeout: float = 15.0
     ssl_verification: bool = True
     ignored_download_clients: list = []
     private_tracker_handling: str = "remove"
@@ -23,6 +24,9 @@ def __init__(self, config):
         self.log_level = general_config.get("log_level", self.log_level.upper())
         self.test_run = general_config.get("test_run", self.test_run)
         self.timer = general_config.get("timer", self.timer)
+        self.request_timeout = general_config.get(
+            "request_timeout", self.request_timeout
+        )
         self.ssl_verification = general_config.get(
             "ssl_verification", self.ssl_verification
         )

src/settings/_jobs.py

@@ -44,6 +44,10 @@ def _remove_none_attributes(self):
             if getattr(self, attr) is None:
                 delattr(self, attr)
 
+    def __bool__(self):
+        """Allow direct truthiness checks to reflect whether this job is enabled."""
+        return bool(getattr(self, "enabled", False))
+
 
 class JobDefaults:
     """Represents default job settings."""

src/settings/_user_config.py

@@ -13,6 +13,7 @@
         "LOG_LEVEL",
         "TEST_RUN",
         "TIMER",
+        "REQUEST_TIMEOUT",
         "SSL_VERIFICATION",
         "IGNORED_DOWNLOAD_CLIENTS",
         "PRIVATE_TRACKER_HANDLING",

src/utils/common.py

@@ -42,7 +42,7 @@ async def make_request(
     method: str,
     endpoint: str,
     settings,
-    timeout: int = 15,
+    timeout: float | None = None,
     *,
     log_error=True,
     **kwargs,
@@ -51,6 +51,11 @@ async def make_request(
     A utility function to make HTTP requests (GET, POST, DELETE, PUT).
     """
     ignore_test_run = kwargs.pop("ignore_test_run", False)
+    request_timeout = timeout
+    if request_timeout is None:
+        request_timeout = getattr(
+            getattr(settings, "general", None), "request_timeout", 15
+        )
 
     if settings.general.test_run and not ignore_test_run:
         if method.lower() in ("put", "post", "delete"):
@@ -74,7 +79,7 @@ async def make_request(
             endpoint,
             **kwargs,
             verify=settings.general.ssl_verification,
-            timeout=timeout,
+            timeout=request_timeout,
         )
         response.raise_for_status()
         return response

tests/jobs/test_job_manager.py

@@ -0,0 +1,77 @@
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+import requests
+
+from src.job_manager import JobManager
+
+
+@pytest.mark.asyncio
+async def test_run_jobs_keeps_running_when_one_group_raises_request_error():
+    settings = MagicMock()
+    manager = JobManager(settings)
+    arr = MagicMock()
+    arr.name = "Sonarr"
+    arr.base_url = "http://sonarr:8989"
+
+    with (
+        patch.object(
+            manager,
+            "removal_jobs",
+            AsyncMock(side_effect=requests.exceptions.ReadTimeout("timed out")),
+        ),
+        patch.object(manager, "search_jobs", AsyncMock()) as search_jobs,
+    ):
+        await manager.run_jobs(arr)
+
+    search_jobs.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_run_download_client_jobs_handles_per_job_request_errors():
+    settings = MagicMock()
+    client = MagicMock()
+    client.name = "qBittorrent"
+    client.base_url = "http://qbittorrent:8080"
+    settings.download_clients.qbittorrent = [client]
+    settings.download_clients.sabnzbd = []
+
+    manager = JobManager(settings)
+    failing_job = MagicMock()
+    failing_job.job.enabled = True
+    failing_job.job_name = "remove_done_seeding"
+    failing_job.run = AsyncMock(
+        side_effect=requests.exceptions.ReadTimeout("timed out")
+    )
+
+    with (
+        patch.object(
+            manager, "_download_clients_connected", AsyncMock(return_value=True)
+        ),
+        patch.object(
+            manager,
+            "_get_download_client_jobs_for_client",
+            MagicMock(return_value=[failing_job]),
+        ),
+    ):
+        result = await manager.run_download_client_jobs()
+
+    assert result == 0
+    failing_job.run.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_run_download_client_jobs_handles_connection_check_request_errors():
+    settings = MagicMock()
+    settings.download_clients.qbittorrent = []
+    settings.download_clients.sabnzbd = []
+    manager = JobManager(settings)
+
+    with patch.object(
+        manager,
+        "_download_clients_connected",
+        AsyncMock(side_effect=requests.exceptions.ReadTimeout("timed out")),
+    ):
+        result = await manager.run_download_client_jobs()
+
+    assert result is None

tests/settings/test_general.py

@@ -0,0 +1,13 @@
+from src.settings._general import General
+
+
+def test_request_timeout_defaults_to_15_seconds():
+    general = General({})
+
+    assert general.request_timeout == 15.0
+
+
+def test_request_timeout_accepts_numeric_strings():
+    general = General({"general": {"request_timeout": "42"}})
+
+    assert general.request_timeout == 42.0