diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 00000000..1c57aa1c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,152 @@ +name: πŸ› Bug Report +description: Report a reproducible problem with decluttarr +title: "[Bug] " +labels: ["Bug"] +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to file a bug. **Please fill in every required field** β€” incomplete reports take much longer to resolve and may be closed pending more info. + + Before submitting, please confirm: + - You've read the ReadMe, especially the **Dependencies & Hints & FAQ** section. + Make sure you read the correct ReadMe version: + + - [**LATEST** ReadMe](https://github.com/ManiMatter/decluttarr/blob/latest/README.md) if you use ```latest``` code + - [**DEV** ReadMe](https://github.com/ManiMatter/decluttarr/blob/dev/README.md) if you use ```dev``` code + - You've searched [open and closed issues](https://github.com/ManiMatter/decluttarr/issues?q=is%3Aissue) for the same problem + - This is a reproducible bug, not a configuration question (use [Discussions](https://github.com/ManiMatter/decluttarr/discussions) for those) + + - type: input + id: version + attributes: + label: Decluttarr commit hash + description: | + The **short commit hash** of the build you're running. `latest` / `dev` tags move too quickly to be useful for reproduction, so please give us the specific commit. + + - **Docker users:** grab it from the `short_commit_id` line in the `πŸ› οΈ Decluttarr - Settings πŸ› οΈ` startup banner (you'll be pasting the full banner in the logs field below) + - **Local Python users:** run `git rev-parse --short HEAD` + placeholder: "e.g. 51b6a4c" + validations: + required: true + + - type: dropdown + id: deployment + attributes: + label: Deployment method + options: + - Docker Compose with config.yaml + - Docker Compose with environment variables + - docker run + - Local Python (main.py) + - Other (describe under "Steps to reproduce") + validations: + required: true + + - type: dropdown + id: os + attributes: + label: Host OS / architecture + description: | + Where decluttarr is running. Path-related bugs (especially `detect_deletions`) and qBit cookie issues sometimes behave differently on Windows or ARM. + options: + - Linux x86_64 (amd64) + - Linux ARM (arm64 / armv7) + - macOS + - Windows + - Other (describe under "Steps to reproduce") + validations: + required: true + + - type: input + id: arr_versions + attributes: + label: "*arr instance versions" + description: Versions of any Sonarr / Radarr / Lidarr / Readarr / Whisparr instances involved in the bug. + placeholder: "e.g. Sonarr 4.0.10, Radarr 5.18.4" + validations: + required: true + + - type: input + id: download_client_version + attributes: + label: Download client + version + description: qBittorrent / SABnzbd / etc. and version. Write `n/a` if the bug doesn't involve a download client. + placeholder: "e.g. qBittorrent 5.2.0, or n/a" + validations: + required: true + + - type: dropdown + id: test_run + attributes: + label: test_run state + description: | + Removal-related bugs behave very differently with `test_run: true` (no actual removals) vs `false`. Pick the value of `general.test_run` (or `TEST_RUN`) when the bug occurred. + options: + - "false (production β€” actual removals)" + - "true (test_run mode β€” no actual removals)" + - Reproduces with both + - N/A (bug doesn't involve removal logic) + validations: + required: true + + - type: dropdown + id: frequency + attributes: + label: Reproduction frequency + options: + - Always (every cycle / every time) + - Sometimes (intermittent) + - Once / haven't tried again + - Unknown + validations: + required: true + + - type: textarea + id: logs + attributes: + label: Decluttarr startup banner + debug logs + description: | + Please follow these three steps to produce a useful log dump: + + 1. **Disable jobs not related to your issue** in your config β€” keeps the logs focused and short + 2. **Set `log_level: DEBUG`** (or `VERBOSE` if DEBUG is too noisy) + 3. **Restart decluttarr and paste everything from the `πŸ› οΈ Decluttarr - Settings πŸ› οΈ` startup banner through the cycle that exhibits the bug** + + The startup banner already contains `image_tag` / `short_commit_id` and the entire active config (general, jobs, instances, download clients) with API keys masked as `*****`. + + **Still sanitize before pasting:** scrub any `?apikey=...` query parameters, `Authorization:` headers, session cookies, and any arr/qBit URLs you'd rather not share. Decluttarr already redacts `X-Api-Key` headers but not query-string keys. + render: shell + validations: + required: true + + - type: textarea + id: reproduce + attributes: + label: Steps to reproduce + placeholder: | + 1. Start decluttarr with the config above + 2. ... + 3. ... + validations: + required: true + + - type: textarea + id: expected + attributes: + label: Expected behavior + validations: + required: true + + - type: textarea + id: actual + attributes: + label: Actual behavior + validations: + required: true + + - type: textarea + id: extra + attributes: + label: Anything else? + description: Screenshots, related issues, what you've already tried, hardware/OS details if relevant, etc. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000..a0faad3c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,10 @@ +blank_issues_enabled: false +contact_links: + - name: πŸ’¬ Questions, usage help, config troubleshooting + url: https://github.com/ManiMatter/decluttarr/discussions + about: | + For general questions, configuration help, or "why is this happening" β€” please use Discussions instead of opening an issue. The issue tracker is reserved for reproducible bugs and feature requests. + - name: πŸ“š README & FAQ + url: https://github.com/ManiMatter/decluttarr#dependencies--hints--faq + about: | + Many common problems (qBit cookie issues, multi-instance YAML format, "no valid arr instances" errors, etc.) are covered in the README's Dependencies & Hints & FAQ section. Please check there first. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 00000000..aa523691 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,41 @@ +name: ✨ Feature Request +description: Suggest a new feature or enhancement +title: "[Feature] " +labels: ["Feature"] +body: + - type: markdown + attributes: + value: | + Thanks for proposing an improvement! Please make sure there isn't already an [open issue](https://github.com/ManiMatter/decluttarr/issues?q=is%3Aissue+is%3Aopen+label%3AFeature) or [discussion](https://github.com/ManiMatter/decluttarr/discussions) covering the same idea. + + - type: textarea + id: problem + attributes: + label: What problem does this solve? + description: Describe the use case or pain point that the feature would address. What are you trying to accomplish that's currently awkward, manual, or impossible? + validations: + required: true + + - type: textarea + id: solution + attributes: + label: Proposed solution + description: | + How would this feature work from the user's perspective? + - What does the config look like? + - What does decluttarr do at runtime? + - Are there new env vars / yaml keys / log messages? + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: Alternatives considered + description: Workarounds you've tried, other tools that solve this differently, or alternative implementations you've thought through. + + - type: textarea + id: extra + attributes: + label: Anything else? + description: Mockups, references, related issues, or willingness to contribute a PR. diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f1dc3215..a3f1ee98 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -57,6 +57,9 @@ jobs: needs: unit-tests runs-on: ubuntu-latest if: github.event_name == 'push' + permissions: + contents: read + packages: write steps: - uses: actions/checkout@v4 with: diff --git a/README.md b/README.md index 9d2d9bc7..2c8a70b2 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,14 @@ _Like this app? Thanks for giving it a_ ⭐️ # **Decluttarr** -**Decluttar V2 was released on Nov 1st, 2025 with _breaking config file changes_.** +_Couple of quick hints:_ + +**1. Are you looking at the right ReadMe?** +Check that the ReadMe version corresponds to the branch you are using (i.e. ```latest``` or ```dev```). Default view on this GitHub repo is ```dev``` branch, but you are most likely using ```latest``` (as per below [instructions](#getting-started)). Thus make sure you change the branch of the ReadMe if thatβ€˜s the case: +- [**LATEST** ReadMe](https://github.com/ManiMatter/decluttarr/blob/latest/README.md) +- [**DEV** ReadMe](https://github.com/ManiMatter/decluttarr/blob/dev/README.md) + +**2. Decluttar V2 was released on Nov 1st, 2025 with _breaking config file changes_.** Looking to **upgrade from V1 to V2**? Look [here](#upgrading-from-v1-to-v2) @@ -20,6 +27,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) @@ -49,6 +57,7 @@ Looking to **upgrade from V1 to V2**? Look [here](#upgrading-from-v1-to-v2) - [READARR](#readarr) - [LIDARR](#lidarr) - [WHISPARR](#whisparr) + - [SPORTARR](#sportarr) - [Downloaders](#download-clients) - [QBITTORRENT](#qbittorrent) @@ -56,7 +65,7 @@ Looking to **upgrade from V1 to V2**? Look [here](#upgrading-from-v1-to-v2) Decluttarr is a helper tool that works with the *arr-application suite, and automates the clean-up for their download queues, keeping them free of stalled / redundant downloads. -It supports [Radarr](https://github.com/Radarr/Radarr/), [Sonarr](https://github.com/Sonarr/Sonarr/), [Readarr](https://github.com/Readarr/Readarr/), [Lidarr](https://github.com/Lidarr/Lidarr/), and [Whisparr](https://github.com/Whisparr/Whisparr/). +It supports [Radarr](https://github.com/Radarr/Radarr/), [Sonarr](https://github.com/Sonarr/Sonarr/), [Sportarr](https://github.com/Sportarr/Sportarr/), [Readarr](https://github.com/Readarr/Readarr/), [Lidarr](https://github.com/Lidarr/Lidarr/), and [Whisparr](https://github.com/Whisparr/Whisparr/). Feature overview: @@ -193,6 +202,7 @@ services: LOG_LEVEL: INFO TEST_RUN: True TIMER: 10 + # REQUEST_TIMEOUT: 15 # IGNORED_DOWNLOAD_CLIENTS: > # - emulerr # SSL_VERIFICATION: true @@ -267,6 +277,10 @@ services: - base_url: "http://sonarr2:8989" api_key: "$SONARR_API_KEY" + # SPORTARR: > + # - base_url: "http://sportarr:1867" + # api_key: "$SPORTARR_API_KEY" + # RADARR: > # - base_url: "http://radarr:7878" # api_key: "$RADARR_API_KEY" @@ -286,8 +300,9 @@ services: # --- Download Clients --- QBITTORRENT: > - base_url: "http://qbittorrent:8080" - # username: "$QBIT_USERNAME" # (optional -> if not provided, assuming not needed) - # password: "$QBIT_PASSWORD" # (optional -> if not provided, assuming not needed) + # api_key: "$QBIT_API_KEY" # (recommended -> requires qBittorrent 5.2.0+; takes precedence over username/password) + # username: "$QBIT_USERNAME" # (legacy -> for qBittorrent < 5.2; ignored if api_key is set) + # password: "$QBIT_PASSWORD" # (legacy -> for qBittorrent < 5.2; ignored if api_key is set) name: "qBittorrent 1" # (optional -> if not provided, assuming "qBittorrent". Must correspond with what is specified in your *arr as download client name) - base_url: "http://qbittorrent:8080" name: "qBittorrent 2" @@ -399,6 +414,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 @@ -422,8 +444,9 @@ Configures the general behavior of the application (across all features) - "remove" means that torrents are removed (default behavior) - "skip" means they are disregarded (which some users might find handy to protect their private trackers prematurely, i.e., before their seed targets are met) - "obsolete_tag" means that rather than being removed, the torrents are tagged. This allows other applications (such as [qbit_manage](https://github.com/StuffAnThings/qbit_manage) to monitor them and remove them once seed targets are fulfilled) + - "remove_from_queue" behaves like "remove" on the *arr side (the queue entry is cleared, and blocklisted if the job blocklists), but leaves the torrent untouched in the download client. Use this when the torrent still carries a hit-and-run obligation you must honour: the *arr stops waiting on a download that will never import and is free to grab an alternative, while seeding continues and another application (such as [qbit_manage](https://github.com/StuffAnThings/qbit_manage)) retires the torrent on the tracker's terms. Unlike "obsolete_tag" this clears the queue entry immediately rather than waiting for the torrent to disappear - Type: String -- Permissible Values: remove, skip, obsolete_tag +- Permissible Values: remove, remove_from_queue, skip, obsolete_tag - Is Mandatory: No (Defaults to remove) @@ -551,11 +574,13 @@ This is the interesting section. It defines which job you want decluttarr to run - Steers whether downloads stuck obtaining metadata are removed from the queue - Blocklisted: Yes - Type: Boolean or Dict -- Permissible Values: True, False or max_strikes (int) +- Permissible Values: True, False or max_strikes (int), detect_via_missing_size (bool) - Is Mandatory: No (Defaults to False) - Note: - With max_strikes you can define how many times this torrent can be caught before being removed - Instead of configuring it here, you may also configure it as a default across all jobs or use the built-in defaults (see further above under "max_strikes") + - By default, this check relies on the "qBittorrent is downloading metadata" message that qBittorrent surfaces in the \*arr queue. Other download clients (e.g. Transmission, Deluge) do not surface such a message, so a torrent stuck fetching metadata stays undetected (see [#57](https://github.com/ManiMatter/decluttarr/issues/57)). + - Set `detect_via_missing_size: true` to additionally flag, regardless of download client, queued items whose size is not yet known (size 0), which is the client-agnostic signature of "no metadata yet". This is debounced by max_strikes. Defaults to False to keep existing (qBittorrent) behavior unchanged. #### REMOVE_MISSING_FILES @@ -679,6 +704,8 @@ Defines arr-instances on which download queue should be decluttered - Equivalent of [Radarr](#radarr) #### Whisparr - Equivalent of [Radarr](#radarr) +#### Sportarr +- Equivalent of [Radarr](#radarr) --- @@ -695,8 +722,9 @@ Supported download clients: **qBittorrent** and **SABnzbd**. - Type: List of qbit instances - Keys per instance - base_url: URL under which the qbit can be reached (mandatory) - - username: Optional - only needed if your qbit requires authentication (which you may not need if you have configured qbit in a way that it disables it for local connections) - - password: Optional - see above + - api_key: Recommended - qBittorrent API key (requires qBittorrent 5.2.0 or newer; generate it under Web UI settings). Authenticates without storing credentials, and takes precedence over username/password if both are set. + - username: Legacy - only for qBittorrent < 5.2, or if your qbit requires authentication (which you may not need if qbit disables it for local connections). Ignored when api_key is set. + - password: Legacy - see above - name: Optional. Needs to correspond with the name that you have set up in your Arr instance. Defaults to "qBittorrent" #### SABNZBD diff --git a/config/config_example.yaml b/config/config_example.yaml index 71ec755d..d00ac73e 100644 --- a/config/config_example.yaml +++ b/config/config_example.yaml @@ -2,10 +2,12 @@ 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 - # public_tracker_handling: "remove" # remove, skip, obsolete_tag. Optional. Default: remove + # request_timeout: 15 # Optional: Request timeout in seconds. Defaults to 15. Can be overridden per instance. + # private_tracker_handling: "obsolete_tag" # remove, remove_from_queue, skip, obsolete_tag. Optional. Default: remove + # public_tracker_handling: "remove" # remove, remove_from_queue, skip, obsolete_tag. Optional. Default: remove # obsolete_tag: "Obsolete" # optional. Default: "Obsolete" # protected_tag: "Keep" # optional. Default: "Keep" @@ -52,6 +54,10 @@ instances: sonarr: - base_url: "http://sonarr:8989" api_key: "xxxx" + # timeout: 30 # Optional: Overrides general timeout for this instance. + # sportarr: + # - base_url: "http://sportarr:1867" + # api_key: "xxxx" radarr: - base_url: "http://radarr:7878" api_key: "xxxx" @@ -68,9 +74,11 @@ instances: download_clients: qbittorrent: - base_url: "http://qbittorrent:8080" # You can use decluttarr without qbit (not all features available, see readme). - # username: xxxx # (optional -> if not provided, assuming not needed) - # password: xxxx # (optional -> if not provided, assuming not needed) + # api_key: "qbt_xxxx" # (recommended -> requires qBittorrent 5.2.0+; generate under Web UI settings. Takes precedence over username/password.) + # username: xxxx # (legacy -> for qBittorrent < 5.2; ignored if api_key is set) + # password: xxxx # (legacy -> for qBittorrent < 5.2; ignored if api_key is set) # name: "qBittorrent" # (optional -> if not provided, assuming "qBittorrent". Must correspond with what is specified in your *arr as download client name) + # timeout: 30 # (optional -> overrides general timeout for this instance) # sabnzbd: # - base_url: "http://sabnzbd:8080" # SABnzbd server URL # api_key: "your_api_key_here" # (required -> SABnzbd API key) diff --git a/main.py b/main.py index 6b989ad5..f08c0d40 100644 --- a/main.py +++ b/main.py @@ -8,7 +8,7 @@ from src.job_manager import JobManager from src.settings.settings import Settings from src.utils.log_setup import logger -from src.utils.startup import launch_steps +from src.utils.startup import launch_steps, retry_degraded_instances settings = Settings() job_manager = JobManager(settings) @@ -52,23 +52,42 @@ async def wait_next_run(): async def main(): await launch_steps(settings) - if settings.jobs.detect_deletions: - await WatcherManager(settings).setup() + if settings.jobs.detect_deletions.enabled: + await watch_manager.setup() # Start Cleaning while True: logger.info("-" * 50) + # Give degraded instances a chance to rejoin before this cycle's jobs + await retry_degraded_instances(settings, watch_manager) + # Refresh qBit Cookies (SABnzbd doesn't need cookie refresh) for qbit in settings.download_clients.qbittorrent: - await qbit.refresh_cookie() + if not qbit.ready: + continue + 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: + if not arr.ready: # skip was already logged by retry_degraded_instances + continue 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() diff --git a/src/deletion_handler/deletion_handler.py b/src/deletion_handler/deletion_handler.py index 2086fd8d..8b180f67 100644 --- a/src/deletion_handler/deletion_handler.py +++ b/src/deletion_handler/deletion_handler.py @@ -99,35 +99,53 @@ async def setup(self): for arr, folder_path in folders_to_watch: self.set_watcher(arr, folder_path) + async def setup_for_arr(self, arr): + """Set up deletion watchers for a single arr (e.g. one that rejoined after a degraded startup).""" + if self.loop is None: + self.loop = asyncio.get_running_loop() + for folder_path in await self.get_folders_to_watch_for_arr(arr): + self.set_watcher(arr, folder_path) + async def get_folders_to_watch(self): """Gets from all arrs the root folders and lists those that are accessible for the arr, and have present for decluttarr.""" folders_to_watch = [] logger.verbose("") logger.verbose("*** Setting up monitoring for deletions ***") for arr in self.settings.instances: - if arr.arr_type not in ( - "sonarr", - "radarr", - ): # only working for sonarr / radarr for now - continue - root_folders = await arr.get_root_folders() - - for folder in root_folders: - if folder.get("accessible") and "path" in folder: - path = Path(folder["path"]) - if path.exists(): - folders_to_watch.append((arr, folder["path"])) - else: - logger.warning( - f"Job 'detect_deletions' on {arr.name} ({arr.base_url}) does not have access to this path and will not monitor it: '{path}'" - ) + for folder_path in await self.get_folders_to_watch_for_arr(arr): + folders_to_watch.append((arr, folder_path)) + + return folders_to_watch + + async def get_folders_to_watch_for_arr(self, arr): + """Root folder paths of one arr that are accessible for both the arr and decluttarr.""" + folders_to_watch = [] + if arr.arr_type not in ( + "sonarr", + "sportarr", + "radarr", + ): # only working for sonarr / radarr for now + return folders_to_watch + if not arr.ready: # degraded instance; watchers are added when it rejoins + return folders_to_watch + root_folders = await arr.get_root_folders() + + for folder in root_folders: + if folder.get("accessible") and "path" in folder: + path = Path(folder["path"]) + if path.exists(): + folders_to_watch.append(folder["path"]) + else: + logger.warning( + f"Job 'detect_deletions' on {arr.name} ({arr.base_url}) does not have access to this path and will not monitor it: '{path}'" + ) + logger.info( + ">>> πŸ’‘ Tip: Make sure that the paths in decluttarr and in your arr instance are identical." + ) + if self.settings.envs.in_docker: logger.info( - ">>> πŸ’‘ Tip: Make sure that the paths in decluttarr and in your arr instance are identical." + ">>> πŸ’‘ Tip: Make sure decluttarr and your arr instance have the same mount points" ) - if self.settings.envs.in_docker: - logger.info( - ">>> πŸ’‘ Tip: Make sure decluttarr and your arr instance have the same mount points" - ) return folders_to_watch diff --git a/src/job_manager.py b/src/job_manager.py index 978d10bb..564f9ba5 100644 --- a/src/job_manager.py +++ b/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 @@ -41,6 +56,9 @@ async def run_download_client_jobs(self): ) for client in download_clients: + if not client.ready: + continue + # Get jobs for this client download_client_jobs = self._get_download_client_jobs_for_client( client, @@ -56,7 +74,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 @@ -116,12 +136,48 @@ async def _queue_has_items(self): "job_runner/full_queue at start: %s", queue_manager.format_queue(full_queue), ) + self._forget_deleted_downloads_still_in_queue(full_queue) return True self.arr.tracker.reset() logger.verbose("Removal Jobs: None triggered (Queue is empty)") return False + def _forget_deleted_downloads_still_in_queue(self, full_queue): + """ + Drop downloads from the deleted tracker that are still in the queue. + + The deleted tracker stops the same download from being removed twice + within one run, but is otherwise only cleared once the queue is empty. + A download that is still in the queue was evidently not removed, so + keeping it in the tracker would silently suppress every future removal + attempt for as long as decluttarr runs. + + Downloads handled via obsolete_tag are exempt: they are meant to stay in + the queue, so their presence is not evidence of a failed removal. + """ + queue_download_ids = {item.get("downloadId") for item in full_queue} - {None} + still_in_queue = { + download_id + for download_id in self.arr.tracker.deleted + if download_id in queue_download_ids + and download_id not in self.arr.tracker.obsolete_tagged + } + if not still_in_queue: + return + + logger.debug( + "job_manager.py/_forget_deleted_downloads_still_in_queue: " + "Marked as deleted but still in queue (removal did not take effect), " + "thus eligible for removal again: %s", + ", ".join(sorted(still_in_queue)), + ) + self.arr.tracker.deleted = [ + download_id + for download_id in self.arr.tracker.deleted + if download_id not in still_in_queue + ] + async def _download_clients_connected(self): for clients in [ self.settings.download_clients.qbittorrent, @@ -133,6 +189,8 @@ async def _download_clients_connected(self): async def _check_client_connection_status(self, clients): for client in clients: + if not client.ready: # never-set-up client must not veto or be polled + continue logger.debug( f"job_manager.py/_check_client_connection_status: Checking if {client.name} is connected", ) @@ -143,6 +201,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. diff --git a/src/jobs/removal_handler.py b/src/jobs/removal_handler.py index 5af6f684..7aec4814 100644 --- a/src/jobs/removal_handler.py +++ b/src/jobs/removal_handler.py @@ -21,8 +21,16 @@ async def remove_downloads(self, affected_downloads, blocklist): if handling_method == "remove": await self._remove_download(affected_download, download_id, blocklist) + elif handling_method == "remove_from_queue": + await self._remove_download( + affected_download, + download_id, + blocklist, + remove_from_client=False, + ) elif handling_method == "obsolete_tag": await self._tag_as_obsolete(affected_download, download_id) + self.arr.tracker.obsolete_tagged.append(download_id) # Print out detailed removal messages (if any) if "removal_messages" in affected_download: @@ -31,19 +39,31 @@ async def remove_downloads(self, affected_downloads, blocklist): self.arr.tracker.deleted.append(download_id) - async def _remove_download(self, affected_download, download_id, blocklist): + async def _remove_download( + self, affected_download, download_id, blocklist, *, remove_from_client=True + ): queue_id = affected_download["queue_ids"][0] + action = "removal" if remove_from_client else "queue removal (torrent kept)" logger.info( - f"Job '{self.job_name}' triggered removal: {affected_download['title']}" + f"Job '{self.job_name}' triggered {action}: {affected_download['title']}" + ) + logger.debug( + f"remove_handler.py/_remove_download: download_id={download_id}, " + f"remove_from_client={remove_from_client}" + ) + await self.arr.remove_queue_item( + queue_id=queue_id, + blocklist=blocklist, + remove_from_client=remove_from_client, ) - logger.debug(f"remove_handler.py/_remove_download: download_id={download_id}") - await self.arr.remove_queue_item(queue_id=queue_id, blocklist=blocklist) async def _tag_as_obsolete(self, affected_download, download_id): logger.info( f"Job '{self.job_name}' triggered obsolete-tagging: {affected_download['title']}" ) for qbit in self.settings.download_clients.qbittorrent: + if not qbit.ready: + continue await qbit.set_tag( tags=[self.settings.general.obsolete_tag], hashes=[download_id] ) @@ -55,12 +75,12 @@ async def _get_handling_method(self, download_id, affected_download): download_client_name = affected_download["downloadClient"] _, download_client_type = ( self.settings.download_clients.get_download_client_by_name( - download_client_name + download_client_name, ready_only=True ) ) if download_client_type != "qbittorrent": - return "remove" # handling is only implemented for qbit + return "remove" # handling is only implemented for qbit (and only if ready) if len(self.settings.download_clients.qbittorrent) == 0: return "remove" # qbit not configured, thus can't tag diff --git a/src/jobs/removal_job.py b/src/jobs/removal_job.py index 3daaec2f..517d86a9 100644 --- a/src/jobs/removal_job.py +++ b/src/jobs/removal_job.py @@ -50,6 +50,7 @@ async def run(self) -> int: # -- Checks -- self._ignore_protected() + self._ignore_degraded_client_downloads() if self.max_strikes: self.affected_downloads = self.strikes_handler.filter_strike_exceeds( self.affected_downloads, self.queue @@ -76,6 +77,36 @@ def _ignore_protected(self): if download_id not in self.arr.tracker.protected } + def _ignore_degraded_client_downloads(self): + """Fail-closed: never remove a download whose configured client is degraded. + + A degraded (not-ready) download client can't be queried for protection + status (protected tag, private/public tracker), so its downloads must be + left untouched rather than deleted blindly. Downloads on healthy clients, + or on clients not configured in decluttarr, are unaffected. + """ + safe = {} + skipped = 0 + for download_id, download in self.affected_downloads.items(): + # download is the grouped metadata dict from group_by_download_id. + client_name = download.get("downloadClient") + client = None + if client_name: + client, _ = self.settings.download_clients.get_download_client_by_name( + client_name + ) + if client is not None and not client.ready: + skipped += 1 + continue + safe[download_id] = download + + if skipped: + logger.warning( + f">>> {self.job_name}: skipped {skipped} download(s) on {self.arr.name} " + "whose download client is degraded (protection can't be verified; left untouched).", + ) + self.affected_downloads = safe + @abstractmethod # Implemented on level of each removal job async def _find_affected_items(self) -> None: pass diff --git a/src/jobs/remove_bad_files.py b/src/jobs/remove_bad_files.py index b3901d7b..2037db67 100644 --- a/src/jobs/remove_bad_files.py +++ b/src/jobs/remove_bad_files.py @@ -62,7 +62,7 @@ def _group_download_ids_by_client(self): download_client, download_client_type = ( self.settings.download_clients.get_download_client_by_name( - download_client_name + download_client_name, ready_only=True ) ) if not download_client or not download_client_type: diff --git a/src/jobs/remove_metadata_missing.py b/src/jobs/remove_metadata_missing.py index 31e86417..7b4503f8 100644 --- a/src/jobs/remove_metadata_missing.py +++ b/src/jobs/remove_metadata_missing.py @@ -2,9 +2,30 @@ class RemoveMetadataMissing(RemovalJob): - queue_scope = "normal" + queue_scope = "full" blocklist = True async def _find_affected_items(self): + # qBittorrent surfaces a dedicated *arr error message while fetching metadata. conditions = [("queued", "qBittorrent is downloading metadata")] - return self.queue_manager.filter_queue(self.queue, conditions) + affected_items = self.queue_manager.filter_queue(self.queue, conditions) + + # Other clients (e.g. Transmission, Deluge) do not surface such a message, so a + # torrent stuck fetching metadata is invisible to the message-based check above + # (see #57). Opt-in fallback: also flag queued items whose size is not yet known + # (size == 0), which is the client-agnostic signature of "no metadata yet". + # Keep this fallback on the normal queue so widening the exact-message check to + # the full queue does not make unrelated full-only size-zero items eligible. + # Debounced by max_strikes like the rest of this job. + if getattr(self.job, "detect_via_missing_size", False): + normal_queue = await self.queue_manager.get_queue_items( + queue_scope="normal" + ) + seen = {(item.get("id"), item.get("downloadId")) for item in affected_items} + for item in self.queue_manager.filter_missing_size(normal_queue): + item_identity = (item.get("id"), item.get("downloadId")) + if item_identity not in seen: + affected_items.append(item) + seen.add(item_identity) + + return affected_items diff --git a/src/jobs/remove_slow.py b/src/jobs/remove_slow.py index 8831ab3b..18de138a 100644 --- a/src/jobs/remove_slow.py +++ b/src/jobs/remove_slow.py @@ -168,7 +168,7 @@ async def add_download_client_to_queue_items(self): download_client_name = item["downloadClient"] download_client, download_client_type = ( self.settings.download_clients.get_download_client_by_name( - download_client_name + download_client_name, ready_only=True ) ) item["download_client"] = download_client diff --git a/src/jobs/remove_unmonitored.py b/src/jobs/remove_unmonitored.py index 8b0d632a..d85599c5 100644 --- a/src/jobs/remove_unmonitored.py +++ b/src/jobs/remove_unmonitored.py @@ -1,4 +1,7 @@ +import requests + from src.jobs.removal_job import RemovalJob +from src.utils.log_setup import logger class RemoveUnmonitored(RemovalJob): @@ -10,7 +13,24 @@ async def _find_affected_items(self): monitored_download_ids = [] for item in self.queue: detail_item_id = item["detail_item_id"] - if detail_item_id is None or await self.arr.is_monitored(detail_item_id): + try: + is_monitored = detail_item_id is None or await self.arr.is_monitored( + detail_item_id + ) + except requests.exceptions.HTTPError as err: + response = getattr(err, "response", None) + if response is None or response.status_code != 404: # noqa: PLR2004 + raise + logger.warning( + "Skipping stale queue item %s: %s %s no longer exists on %s.", + item.get("downloadId"), + self.arr.detail_item_key, + detail_item_id, + self.arr.name, + ) + is_monitored = True + + if is_monitored: # When queue item has been matched to artist (for instance in lidarr) but not yet to the detail (eg. album), then detail key is logically missing. # Thus we can't check if the item is monitored yet monitored_download_ids.append(item["downloadId"]) diff --git a/src/jobs/search_handler.py b/src/jobs/search_handler.py index 671ab535..602f6393 100644 --- a/src/jobs/search_handler.py +++ b/src/jobs/search_handler.py @@ -122,7 +122,7 @@ async def _log_items(self, items): title = item.get("title", "Unknown") logger.verbose(f"- {title}") - elif self.arr.arr_type == "sonarr": + elif self.arr.arr_type in ("sonarr", "sportarr"): logger.debug( "search_handler.py/_log_items: Getting series information for better display in output" ) diff --git a/src/settings/_constants.py b/src/settings/_constants.py index 904560e0..ed8a60bc 100644 --- a/src/settings/_constants.py +++ b/src/settings/_constants.py @@ -22,6 +22,7 @@ class Paths: class ApiEndpoints: radarr = "/api/v3" sonarr = "/api/v3" + sportarr = "/api/v3" lidarr = "/api/v1" readarr = "/api/v1" whisparr = "/api/v3" @@ -31,6 +32,7 @@ class ApiEndpoints: class MinVersions: radarr = "5.10.3.9171" sonarr = "4.0.9.2332" + sportarr = "4.0.0.0" lidarr = "2.11.1.4621" readarr = "0.4.15.2787" whisparr = "2.0.0.548" @@ -41,6 +43,7 @@ class MinVersions: class FullQueueParameter: radarr = "includeUnknownMovieItems" sonarr = "includeUnknownSeriesItems" + sportarr = "includeUnknownSeriesItems" lidarr = "includeUnknownArtistItems" readarr = "includeUnknownAuthorItems" whisparr = "includeUnknownSeriesItems" @@ -49,6 +52,7 @@ class FullQueueParameter: class DetailItemKey: radarr = "movie" sonarr = "episode" + sportarr = "episode" lidarr = "album" readarr = "book" whisparr = "episode" @@ -57,6 +61,7 @@ class DetailItemKey: class DetailItemSearchCommand: radarr = "MoviesSearch" sonarr = "EpisodeSearch" + sportarr = "EpisodeSearch" lidarr = "AlbumSearch" readarr = "BookSearch" whisparr = None @@ -65,8 +70,10 @@ class DetailItemSearchCommand: class RefreshItemKey: radarr = "movie" sonarr = "series" + sportarr = "series" class RefreshItemCommand: radarr = "RefreshMovie" sonarr = "RefreshSeries" + sportarr = "RefreshSeries" diff --git a/src/settings/_download_clients.py b/src/settings/_download_clients.py index d5d6b5e9..bba0e0d3 100644 --- a/src/settings/_download_clients.py +++ b/src/settings/_download_clients.py @@ -81,11 +81,16 @@ def check_unique_download_client_types(self): seen.add(name.lower()) def get_download_client_by_name( - self, name: str, download_client_type: str | None = None + self, + name: str, + download_client_type: str | None = None, + ready_only: bool = False, ): """ Retrieve the download client and download client type by its name. If download_client_type is provided, search only in that type. + If ready_only is True, a client that failed its startup check (not ready) + is treated as unavailable, so degraded clients never get called from jobs. """ name_lower = name.lower() types_to_search = ( @@ -97,6 +102,8 @@ def get_download_client_by_name( for download_client in download_clients: if download_client.name.lower() == name_lower: + if ready_only and not getattr(download_client, "ready", False): + return None, None return download_client, client_type return None, None diff --git a/src/settings/_download_clients_qbit.py b/src/settings/_download_clients_qbit.py index 431a7b5f..8d6896f1 100644 --- a/src/settings/_download_clients_qbit.py +++ b/src/settings/_download_clients_qbit.py @@ -2,12 +2,19 @@ from requests.cookies import RequestsCookieJar from src.settings._constants import ApiEndpoints, MinVersions -from src.utils.common import extract_json_from_response, make_request, wait_and_exit +from src.utils.common import ( + extract_json_from_response, + is_definitive_setup_error, + make_request, +) from src.utils.log_setup import logger class QbitError(Exception): - pass + def __init__(self, message, tip="", definitive=False): + super().__init__(message) + self.tip = tip + self.definitive = definitive class QbitClients(list): @@ -39,6 +46,10 @@ class QbitClient: cookie: dict[str, str] = None version: str = None bandwidth_usage: int = 0 + ready: bool = False + failure_kind: str = None # None | "transient" | "definitive" + last_error: str = None + setup_tip: str = "" def __init__( self, @@ -47,8 +58,11 @@ def __init__( username: str = None, password: str = None, name: str = None, + timeout: int | None = None, + api_key: str = None, ): self.settings = settings + self._timeout = timeout if not base_url: logger.error("Skipping qBittorrent client entry: 'base_url' is required.") error = "qBittorrent client must have a 'base_url'." @@ -59,6 +73,8 @@ def __init__( self.min_version = MinVersions.qbittorrent self.username = username self.password = password + # Treat an empty/whitespace api_key as unset so it falls back to password auth. + self.api_key = api_key.strip() if isinstance(api_key, str) else api_key self.name = name if not self.name: logger.verbose( @@ -66,16 +82,47 @@ def __init__( ) self.name = "qBittorrent" + if self.api_key and (username or password): + logger.info( + f"qBittorrent '{self.name}': both api_key and username/password provided; using api_key (credentials ignored).", + ) + self._remove_none_attributes() + @property + def timeout(self): + instance_timeout = getattr(self, "_timeout", None) + if instance_timeout is not None: + return instance_timeout + return getattr(getattr(self.settings, "general", None), "request_timeout", 15) + def _remove_none_attributes(self): """Remove attributes that are None to keep the object clean.""" for attr in list(vars(self)): if getattr(self, attr) is None: delattr(self, attr) + def _auth_kwargs(self) -> dict: + """Return the make_request auth kwargs for the configured mode. + + Key mode (qBit >= 5.2): stateless 'Authorization: Bearer' header on every + request. Password mode (legacy): the session SID cookie from refresh_cookie(). + Must stay a method (not cached) so password mode reads the freshly + refreshed self.cookie each cycle. + """ + api_key = getattr(self, "api_key", None) + if api_key: + return {"headers": {"Authorization": f"Bearer {api_key}"}} + return {"cookies": getattr(self, "cookie", None)} + async def refresh_cookie(self): """Refresh the qBittorrent session cookie.""" + if getattr(self, "api_key", None): + # Key mode is stateless; qBit rejects /auth/login under API-key auth. + logger.debug( + "_download_clients_qBit.py/refresh_cookie: API-key mode, skipping login", + ) + return def _connection_error(): error = "Login failed." @@ -95,6 +142,7 @@ def _connection_error(): "post", endpoint, self.settings, + timeout=self.timeout, data=data, headers=headers, ignore_test_run=True, @@ -133,7 +181,8 @@ async def fetch_version(self): "get", endpoint, self.settings, - cookies=self.cookie, + timeout=self.timeout, + **self._auth_kwargs(), ) self.version = response.text[1:] # Remove the '_v' prefix logger.debug( @@ -149,19 +198,44 @@ async def validate_version(self): f"Please update qBittorrent to at least version {min_version}. Current version: {self.version}", ) error = f"qBittorrent version {self.version} is too old. Please update." - raise QbitError(error) + raise QbitError(error, definitive=True) if version.parse(self.version) < version.parse("5.0.0"): logger.info( "[Tip!] Consider upgrading to qBittorrent v5.0.0 or newer to reduce network overhead.", ) + def log_auth_version_guidance(self): + """Log authentication guidance once the qBittorrent version is known.""" + qbit_version = version.parse(self.version) + api_key = getattr(self, "api_key", None) + + if api_key and qbit_version < version.parse("5.2.0"): + logger.warning( + "qBittorrent %s does not support API-key authentication; API keys " + "require qBittorrent 5.2.0 or newer. The configured API key did not " + "authenticate this connection.", + self.version, + ) + elif ( + not api_key + and (getattr(self, "username", None) or getattr(self, "password", None)) + and qbit_version >= version.parse("5.2.0") + ): + logger.info( + "[Tip!] qBittorrent %s supports API-key authentication. Consider " + "replacing username/password with api_key.", + self.version, + ) + async def create_tag(self, tag: str): """Ensure a tag exists in qBittorrent; create it if it doesn't.""" logger.debug( "_download_clients_qBit.py/create_tag: Checking if tag '{tag}' exists (and creating it if not)", ) url = f"{self.api_url}/torrents/tags" - response = await make_request("get", url, self.settings, cookies=self.cookie) + response = await make_request( + "get", url, self.settings, timeout=self.timeout, **self._auth_kwargs() + ) current_tags = response.json() if tag not in current_tags: @@ -171,8 +245,9 @@ async def create_tag(self, tag: str): "post", self.api_url + "/torrents/createTags", self.settings, + timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def create_required_tags(self): @@ -196,7 +271,8 @@ async def set_unwanted_folder(self): "get", endpoint, self.settings, - cookies=self.cookie, + timeout=self.timeout, + **self._auth_kwargs(), ) qbit_settings = response.json() @@ -209,77 +285,131 @@ async def set_unwanted_folder(self): "post", self.api_url + "/app/setPreferences", self.settings, + timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def check_qbit_reachability(self): - """Check if the qBittorrent URL is reachable.""" + """Check if the qBittorrent URL is reachable (and the credentials work).""" try: logger.debug( "_download_clients_qBit.py/check_qbit_reachability: Checking if qbit is reachable", ) - endpoint = f"{self.api_url}/auth/login" - data = { - "username": getattr(self, "username", ""), - "password": getattr(self, "password", ""), - } - headers = {"content-type": "application/x-www-form-urlencoded"} - await make_request( - "post", - endpoint, - self.settings, - data=data, - headers=headers, - log_error=False, - ignore_test_run=True, - ) - + if getattr(self, "api_key", None): + # Key mode: qBit rejects /auth/login, so probe a normal authed + # endpoint with the Bearer header instead. + await make_request( + "get", + f"{self.api_url}/app/version", + self.settings, + timeout=self.timeout, + log_error=False, + ignore_test_run=True, + **self._auth_kwargs(), + ) + response = None # key mode: no login body to inspect below + else: + endpoint = f"{self.api_url}/auth/login" + data = { + "username": getattr(self, "username", ""), + "password": getattr(self, "password", ""), + } + headers = {"content-type": "application/x-www-form-urlencoded"} + response = await make_request( + "post", + endpoint, + self.settings, + timeout=self.timeout, + data=data, + headers=headers, + log_error=False, + ignore_test_run=True, + ) except Exception as e: # noqa: BLE001 - tip = "πŸ’‘ Tip: Did you specify the URL (and username/password if required) correctly?" - logger.error(f"-- | qBittorrent\n❗️ {e}\n{tip}\n") - wait_and_exit() + if getattr(self, "api_key", None): + tip = ( + "πŸ’‘ Tip: Is the qBittorrent API key correct, and is your qBittorrent " + "at least v5.2.0? API-key auth requires qBit 5.2+ (WebAPI 2.14.1+)." + ) + else: + tip = "πŸ’‘ Tip: Did you specify the URL (and username/password if required) correctly?" + if str(e) != self.last_error: # Only report new failure modes in full + logger.error(f"-- | qBittorrent\n❗️ {e}\n{tip}\n") + raise QbitError(e, tip=tip) from e + + # Bad credentials: qBit answers HTTP 200 with the body "Fails." Treat this + # as a definitive config error so we don't retry-loop every cycle and get + # the source IP banned by qBittorrent's failed-login protection. + if getattr(response, "text", None) == "Fails.": + tip = "πŸ’‘ Tip: Check the qBittorrent username/password." + error = "qBittorrent login failed (incorrect username/password)." + if error != self.last_error: + logger.error(f"-- | qBittorrent\n❗️ {error}\n{tip}\n") + raise QbitError(error, tip=tip, definitive=True) async def check_connected(self): """Check if the qBittorrent is connected to internet.""" logger.debug( "_download_clients_qBit.py/check_qbit_reachability: Checking if qbit is connected to the internet", ) - qbit_connection_status = ( - ( - await make_request( - "get", - self.api_url + "/sync/maindata", - self.settings, - cookies=self.cookie, - ) - ).json() - )["server_state"]["connection_status"] + try: + qbit_connection_status = ( + ( + await make_request( + "get", + self.api_url + "/sync/maindata", + self.settings, + timeout=self.timeout, + **self._auth_kwargs(), + ) + ).json() + )["server_state"]["connection_status"] + except Exception as e: + logger.warning( + ">>> %s: Failed to reach /sync/maindata (%s). Treating as disconnected.", + self.name, + e, + ) + return False if qbit_connection_status == "disconnected": return False return True async def setup(self): - """Perform the qBittorrent setup by calling relevant managers.""" - # Check reachabilty - await self.check_qbit_reachability() + """Perform the qBittorrent setup; degrade instead of exiting on failure.""" + try: + await self.check_qbit_reachability() - # Refresh the qBittorrent cookie first - await self.refresh_cookie() + # Refresh the qBittorrent cookie first + await self.refresh_cookie() - try: # Fetch version and validate it await self.fetch_version() await self.validate_version() - logger.info(f"OK | qBittorrent ({self.base_url})") - except QbitError as e: - logger.error(f"qBittorrent version check failed: {e}") - wait_and_exit() # Exit if version check fails + self.log_auth_version_guidance() - # Continue with other setup tasks regardless of version check result - await self.create_required_tags() - await self.set_unwanted_folder() - await self.warn_no_bandwidth_limit_set() + await self.create_required_tags() + await self.set_unwanted_folder() + await self.warn_no_bandwidth_limit_set() + + logger.info(f"OK | qBittorrent ({self.base_url})") + self.ready = True + self.failure_kind = None + self.last_error = None + self.setup_tip = "" + except Exception as e: # noqa: BLE001 + if not isinstance(e, QbitError) and str(e) != self.last_error: + logger.error( + f"Unhandled error during qBittorrent setup: {e}", exc_info=True + ) + self.ready = False + self.failure_kind = ( + "definitive" if is_definitive_setup_error(e) else "transient" + ) + self.last_error = str(e) + self.setup_tip = getattr(e, "tip", "") + return self.ready async def get_protected_and_private(self): """Fetch torrents from qBittorrent and checks for protected and private status.""" @@ -353,8 +483,9 @@ async def set_tag(self, tags, hashes): "post", self.api_url + "/torrents/addTags", self.settings, + timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def fetch_download_progress(self, download_id): @@ -376,8 +507,9 @@ async def get_qbit_items(self, hashes: list[str] | str | None = None) -> list[di method="get", endpoint=f"{self.api_url}/torrents/info", settings=self.settings, + timeout=self.timeout, params=None, # Retrieve all torrents - cookies=self.cookie, + **self._auth_kwargs(), ) all_items = response.json() @@ -395,15 +527,15 @@ async def get_qbit_items(self, hashes: list[str] | str | None = None) -> list[di async def get_torrent_properties(self, qbit_hash): params = {"hash": qbit_hash.lower()} response = await make_request( - "get", - self.api_url + "/torrents/properties", - self.settings, - params=params, - cookies=self.cookie, - ) + "get", + self.api_url + "/torrents/properties", + self.settings, + timeout=self.timeout, + params=params, + **self._auth_kwargs(), + ) return response.json() - async def get_torrent_files(self, download_id): # this may not work if the wrong qbit logger.debug("_download_clients_qBit/get_torrent_files: Getting torrent files") @@ -411,8 +543,9 @@ async def get_torrent_files(self, download_id): method="get", endpoint=self.api_url + "/torrents/files", settings=self.settings, + timeout=self.timeout, params={"hash": download_id.lower()}, - cookies=self.cookie, + **self._auth_kwargs(), ) return response.json() @@ -429,8 +562,9 @@ async def set_torrent_file_priority(self, download_id, file_id, priority=0): "post", self.api_url + "/torrents/filePrio", self.settings, + timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def set_bandwidth_usage(self): @@ -440,7 +574,8 @@ async def set_bandwidth_usage(self): method="get", endpoint=self.api_url + "/transfer/info", settings=self.settings, - cookies=self.cookie, + timeout=self.timeout, + **self._auth_kwargs(), ) records = extract_json_from_response(response) limit = records["dl_rate_limit"] @@ -476,6 +611,7 @@ async def remove_download(self, download_hash: str, delete_files: bool = True): "post", f"{self.api_url}/torrents/delete", self.settings, + timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) diff --git a/src/settings/_download_clients_sabnzbd.py b/src/settings/_download_clients_sabnzbd.py index 49184e58..683d2f54 100644 --- a/src/settings/_download_clients_sabnzbd.py +++ b/src/settings/_download_clients_sabnzbd.py @@ -1,12 +1,15 @@ from packaging import version from src.settings._constants import MinVersions -from src.utils.common import make_request, wait_and_exit +from src.utils.common import is_definitive_setup_error, make_request from src.utils.log_setup import logger class SabnzbdError(Exception): - pass + def __init__(self, message, tip="", definitive=False): + super().__init__(message) + self.tip = tip + self.definitive = definitive class SabnzbdClients(list): @@ -36,6 +39,10 @@ class SabnzbdClient: """Represents a single SABnzbd client.""" version: str = None + ready: bool = False + failure_kind: str = None # None | "transient" | "definitive" + last_error: str = None + setup_tip: str = "" def __init__( self, @@ -43,8 +50,10 @@ def __init__( base_url: str = None, api_key: str = None, name: str = None, + timeout: int | None = None, ): self.settings = settings + self._timeout = timeout if not base_url: logger.error("Skipping SABnzbd client entry: 'base_url' is required.") error = "SABnzbd client must have a 'base_url'." @@ -68,6 +77,13 @@ def __init__( self._remove_none_attributes() + @property + def timeout(self): + instance_timeout = getattr(self, "_timeout", None) + if instance_timeout is not None: + return instance_timeout + return getattr(getattr(self.settings, "general", None), "request_timeout", 15) + def _remove_none_attributes(self): """Remove attributes that are None to keep the object clean.""" for attr in list(vars(self)): @@ -80,7 +96,7 @@ async def fetch_version(self): "_download_clients_sabnzbd.py/fetch_version: Getting SABnzbd Version" ) params = {"mode": "version", "apikey": self.api_key, "output": "json"} - response = await make_request("get", self.api_url, self.settings, params=params) + response = await make_request("get", self.api_url, self.settings, timeout=self.timeout, params=params) response_data = response.json() self.version = response_data.get("version", "unknown") logger.debug( @@ -96,28 +112,42 @@ async def validate_version(self): f"Please update SABnzbd to at least version {min_version}. Current version: {self.version}", ) error = f"SABnzbd version {self.version} is too old. Please update." - raise SabnzbdError(error) + raise SabnzbdError(error, definitive=True) async def check_sabnzbd_reachability(self): - """Check if the SABnzbd URL is reachable.""" + """Check if the SABnzbd URL is reachable (and the API key works).""" try: logger.debug( "_download_clients_sabnzbd.py/check_sabnzbd_reachability: Checking if SABnzbd is reachable" ) params = {"mode": "version", "apikey": self.api_key, "output": "json"} - await make_request( + response = await make_request( "get", self.api_url, self.settings, + timeout=self.timeout, params=params, log_error=False, ignore_test_run=True, ) - except Exception as e: # noqa: BLE001 tip = "πŸ’‘ Tip: Did you specify the URL and API key correctly?" - logger.error(f"-- | SABnzbd\n❗️ {e}\n{tip}\n") - wait_and_exit() + if str(e) != self.last_error: # Only report new failure modes in full + logger.error(f"-- | SABnzbd\n❗️ {e}\n{tip}\n") + raise SabnzbdError(e, tip=tip) from e + + # Bad API key: SABnzbd answers HTTP 200 with {"error": "API Key Incorrect"}. + # Treat as a definitive config error so we don't retry-loop forever. + try: + error_msg = response.json().get("error") + except (ValueError, AttributeError): + error_msg = None + if error_msg: + tip = "πŸ’‘ Tip: Check the SABnzbd API key." + error = f"SABnzbd rejected the request: {error_msg}" + if error != self.last_error: + logger.error(f"-- | SABnzbd\n❗️ {error}\n{tip}\n") + raise SabnzbdError(error, tip=tip, definitive=True) async def check_connected(self): """Check if SABnzbd is connected and operational.""" @@ -129,6 +159,7 @@ async def check_connected(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) status_data = response.json() @@ -137,18 +168,31 @@ async def check_connected(self): return "status" in status_data async def setup(self): - """Perform the SABnzbd setup by calling relevant managers.""" - # Check reachability - await self.check_sabnzbd_reachability() - + """Perform the SABnzbd setup; degrade instead of exiting on failure.""" try: + await self.check_sabnzbd_reachability() + # Fetch version and validate it await self.fetch_version() await self.validate_version() + logger.info(f"OK | SABnzbd ({self.base_url})") - except SabnzbdError as e: - logger.error(f"SABnzbd version check failed: {e}") - wait_and_exit() # Exit if version check fails + self.ready = True + self.failure_kind = None + self.last_error = None + self.setup_tip = "" + except Exception as e: # noqa: BLE001 + if not isinstance(e, SabnzbdError) and str(e) != self.last_error: + logger.error( + f"Unhandled error during SABnzbd setup: {e}", exc_info=True + ) + self.ready = False + self.failure_kind = ( + "definitive" if is_definitive_setup_error(e) else "transient" + ) + self.last_error = str(e) + self.setup_tip = getattr(e, "tip", "") + return self.ready async def get_queue_items(self): """Fetch queue items from SABnzbd.""" @@ -160,6 +204,7 @@ async def get_queue_items(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) queue_data = response.json() @@ -175,6 +220,7 @@ async def get_history_items(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) history_data = response.json() @@ -196,6 +242,7 @@ async def remove_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -215,6 +262,7 @@ async def pause_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -234,6 +282,7 @@ async def resume_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -252,6 +301,7 @@ async def retry_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -308,6 +358,7 @@ async def get_download_speed(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) status_data = response.json() diff --git a/src/settings/_general.py b/src/settings/_general.py index ccd42a6a..c9750950 100644 --- a/src/settings/_general.py +++ b/src/settings/_general.py @@ -2,7 +2,7 @@ from src.settings._validate_data_types import validate_data_types from src.utils.log_setup import logger -VALID_TRACKER_HANDLING = {"remove", "skip", "obsolete_tag"} +VALID_TRACKER_HANDLING = {"remove", "remove_from_queue", "skip", "obsolete_tag"} class General: @@ -11,7 +11,9 @@ class General: log_level: str = "INFO" test_run: bool = False timer: float = 10.0 + request_timeout: float = 15.0 ssl_verification: bool = True + request_timeout: int = 15 ignored_download_clients: list = [] private_tracker_handling: str = "remove" public_tracker_handling: str = "remove" @@ -23,9 +25,13 @@ 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 ) + self.request_timeout = general_config.get("request_timeout", self.request_timeout) self.ignored_download_clients = general_config.get( "ignored_download_clients", self.ignored_download_clients ) diff --git a/src/settings/_instances.py b/src/settings/_instances.py index 69078ff2..d04b77bc 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -13,7 +13,12 @@ RefreshItemCommand, RefreshItemKey, ) -from src.utils.common import extract_json_from_response, make_request, wait_and_exit +from src.utils.common import ( + extract_json_from_response, + is_definitive_setup_error, + make_request, + wait_and_exit, +) from src.utils.log_setup import logger @@ -24,6 +29,7 @@ def __init__(self): self.defective = {} self.download_progress = {} self.deleted = [] + self.obsolete_tagged = [] self.extension_checked = [] def reset(self) -> None: @@ -33,6 +39,7 @@ def reset(self) -> None: self.defective, self.download_progress, self.deleted, + self.obsolete_tagged, self.extension_checked, ): attr.clear() @@ -42,6 +49,8 @@ async def refresh_private_and_protected(self, settings): private_downloads = [] for qbit in settings.download_clients.qbittorrent: + if not qbit.ready: + continue protected, private = await qbit.get_protected_and_private() protected_downloads.extend(protected) private_downloads.extend(private) @@ -51,7 +60,10 @@ async def refresh_private_and_protected(self, settings): class ArrError(Exception): - pass + def __init__(self, message, tip="", definitive=False): + super().__init__(message) + self.tip = tip + self.definitive = definitive class ArrInstances(list): @@ -81,7 +93,7 @@ def config_as_yaml(self, *, hide_internal_attr=True): } outputs = [] - for arr_type in ["sonarr", "radarr", "readarr", "lidarr", "whisparr"]: + for arr_type in ["sonarr", "sportarr", "radarr", "readarr", "lidarr", "whisparr"]: arrs = self.get_by_arr_type(arr_type) if arrs: output = get_config_as_yaml( @@ -122,6 +134,7 @@ def _load_clients(self, config, settings): arr_type=arr_type, base_url=client_config["base_url"], api_key=client_config["api_key"], + timeout=client_config.get("timeout"), ), ) except KeyError as e: @@ -134,8 +147,12 @@ class ArrInstance: version: str = None name: str = None + ready: bool = False + failure_kind: str = None # None | "transient" | "definitive" + last_error: str = None + setup_tip: str = "" - def __init__(self, settings, arr_type: str, base_url: str, api_key: str): + def __init__(self, settings, arr_type: str, base_url: str, api_key: str, timeout: int | None = None): if not base_url: logger.error(f"Skipping {arr_type} client entry: 'base_url' is required.") error = f"{arr_type} client must have a 'base_url'." @@ -147,6 +164,7 @@ def __init__(self, settings, arr_type: str, base_url: str, api_key: str): raise ValueError(error) self.settings = settings + self._timeout = timeout self.tracker = Tracker() self.arr_type = arr_type self.base_url = base_url.rstrip("/") @@ -158,16 +176,22 @@ def __init__(self, settings, arr_type: str, base_url: str, api_key: str): self.detail_item_id_key = self.detail_item_key + "Id" self.detail_item_ids_key = self.detail_item_key + "Ids" self.detail_item_search_command = getattr(DetailItemSearchCommand, arr_type) - if self.arr_type in ("radarr", "sonarr"): + if self.arr_type in ("radarr", "sonarr", "sportarr"): self.refresh_item_key = getattr(RefreshItemKey, arr_type) self.refresh_item_id_key = self.refresh_item_key + "Id" self.refresh_item_command = getattr(RefreshItemCommand, arr_type) + @property + def timeout(self): + if self._timeout is not None: + return self._timeout + return getattr(getattr(self.settings, "general", None), "request_timeout", 15) + async def _check_ui_language(self): """Check if the UI language is set to English.""" endpoint = self.api_url + "/config/ui" headers = {"X-Api-Key": self.api_key} - response = await make_request("get", endpoint, self.settings, headers=headers) + response = await make_request("get", endpoint, self.settings, timeout=self.timeout, headers=headers) ui_language = (response.json())["uiLanguage"] if ui_language > 1: # Not English logger.error("!! %s Error: !!", self.name) @@ -178,7 +202,8 @@ async def _check_ui_language(self): "> Details: https://github.com/ManiMatter/decluttarr/issues/132)", ) error = "Not English" - raise ArrError(error) + tip = f"πŸ’‘ Tip: Set the UI language to English in {self.name} (under Settings/UI)" + raise ArrError(error, tip=tip, definitive=True) def _check_min_version(self, status): """Check if ARR instance meets minimum version requirements.""" @@ -196,6 +221,13 @@ def _check_min_version(self, status): def _check_arr_type(self, status): """Check if the ARR instance is of the correct type.""" actual_arr_type = status["appName"] + if self.arr_type == "sportarr": + # Sportarr's Sonarr-compatible API reports appName "Sonarr" (other + # consumers validate on that), so its identity marker is the extra + # sportarrVersion field a real Sonarr never sends. + if "sportarrVersion" in status: + return + actual_arr_type = "Sonarr" if actual_arr_type.lower() != self.arr_type: logger.error("!! %s Error: !!", self.name) logger.error( @@ -216,6 +248,7 @@ async def _check_reachability(self): "get", endpoint, self.settings, + timeout=self.timeout, headers=headers, log_error=False, ) @@ -234,11 +267,12 @@ async def _check_reachability(self): else: tip = "" - logger.error(f"-- | {self.arr_type} ({self.base_url})\n❗️ {e}\n{tip}\n") - raise ArrError(e) from e + if str(e) != self.last_error: # Only report new failure modes in full + logger.error(f"-- | {self.arr_type} ({self.base_url})\n❗️ {e}\n{tip}\n") + raise ArrError(e, tip=tip) from e async def setup(self): - """Check on specific ARR instance.""" + """Check on specific ARR instance; degrade instead of exiting on failure.""" try: status = await self._check_reachability() self.name = status.get("instanceName", self.arr_type) @@ -251,10 +285,20 @@ async def setup(self): logger.debug(f"Current version of {self.name}: {self.version}") await self._check_matching_decluttarr_download_clients() + self.ready = True + self.failure_kind = None + self.last_error = None + self.setup_tip = "" except Exception as e: # noqa: BLE001 - if not isinstance(e, ArrError): + if not isinstance(e, ArrError) and str(e) != self.last_error: logger.error(f"Unhandled error: {e}", exc_info=True) - wait_and_exit() + self.ready = False + self.failure_kind = ( + "definitive" if is_definitive_setup_error(e) else "transient" + ) + self.last_error = str(e) + self.setup_tip = getattr(e, "tip", "") + return self.ready async def fetch_arr_download_clients(self) -> list[dict[str, object]]: """Fetch the list of download clients from the *arr API.""" @@ -264,7 +308,7 @@ async def fetch_arr_download_clients(self) -> list[dict[str, object]]: endpoint = self.api_url + "/downloadclient" headers = {"X-Api-Key": self.api_key} - response = await make_request("get", endpoint, self.settings, headers=headers) + response = await make_request("get", endpoint, self.settings, timeout=self.timeout, headers=headers) return extract_json_from_response(response) async def _check_matching_decluttarr_download_clients(self): @@ -306,7 +350,9 @@ async def _check_matching_decluttarr_download_clients(self): logger.info(tip) return - async def remove_queue_item(self, queue_id, *, blocklist=False): + async def remove_queue_item( + self, queue_id, *, blocklist=False, remove_from_client=True + ): """ Remove a specific queue item from the queue by its queue id. @@ -315,23 +361,29 @@ async def remove_queue_item(self, queue_id, *, blocklist=False): Args: queue_id (str): The queue ID of the queue item to be removed. blocklist (bool): Whether to add the item to the blocklist. Default is False. + remove_from_client (bool): Whether to also delete the download from the + download client. Default is True. Set to False to clear only the queue + entry and leave the torrent in place (e.g. to keep seeding a private + tracker torrent that still carries a hit-and-run obligation). Returns: bool: Returns True if the removal was successful, False otherwise. """ logger.debug( - f"_instances.py/remove_queue_item: Removing queue item, blocklist: {blocklist}" + f"_instances.py/remove_queue_item: Removing queue item, blocklist: {blocklist}, " + f"remove_from_client: {remove_from_client}" ) endpoint = f"{self.api_url}/queue/{queue_id}" headers = {"X-Api-Key": self.api_key} - query = {"removeFromClient": True, "blocklist": blocklist} + query = {"removeFromClient": remove_from_client, "blocklist": blocklist} # Send the request to remove the download from the queue response = await make_request( "delete", endpoint, self.settings, + timeout=self.timeout, headers=headers, params=query, ) @@ -345,27 +397,27 @@ async def is_monitored(self, detail_id): endpoint = f"{self.api_url}/{self.detail_item_key}/{detail_id}" headers = {"X-Api-Key": self.api_key} - response = await make_request("get", endpoint, self.settings, headers=headers) + response = await make_request("get", endpoint, self.settings, timeout=self.timeout, headers=headers) return response.json()["monitored"] async def get_series(self): """Fetch download client information and return the implementation value.""" endpoint = self.api_url + "/series" headers = {"X-Api-Key": self.api_key} - response = await make_request("get", endpoint, self.settings, headers=headers) + response = await make_request("get", endpoint, self.settings, timeout=self.timeout, headers=headers) return response.json() async def get_root_folders(self): """Fetch Root folders.""" endpoint = self.api_url + "/rootFolder" headers = {"X-Api-Key": self.api_key} - response = await make_request("get", endpoint, self.settings, headers=headers) + response = await make_request("get", endpoint, self.settings, timeout=self.timeout, headers=headers) return response.json() async def get_refresh_item(self): endpoint = self.api_url + "/" + self.refresh_item_key headers = {"X-Api-Key": self.api_key} - response = await make_request("get", endpoint, self.settings, headers=headers) + response = await make_request("get", endpoint, self.settings, timeout=self.timeout, headers=headers) return response.json() async def get_refresh_item_by_path(self, folder_path): @@ -383,6 +435,7 @@ async def refresh_item(self, refresh_item_id): method="POST", endpoint=f"{self.api_url}/command", settings=self.settings, + timeout=self.timeout, json={ "name": self.refresh_item_command, self.refresh_item_id_key: refresh_item_id, diff --git a/src/settings/_jobs.py b/src/settings/_jobs.py index c71eaabc..71d39abb 100644 --- a/src/settings/_jobs.py +++ b/src/settings/_jobs.py @@ -14,6 +14,7 @@ class JobParams: max_concurrent_searches: int min_days_between_searches: int target_tags: list + detect_via_missing_size: bool = False def __init__( self, @@ -25,6 +26,7 @@ def __init__( max_concurrent_searches=None, min_days_between_searches=None, target_tags=None, + detect_via_missing_size=None, ): self.enabled = enabled self.keep_archives = keep_archives @@ -34,6 +36,7 @@ def __init__( self.max_concurrent_searches = max_concurrent_searches self.min_days_between_searches = min_days_between_searches self.target_tags = target_tags + self.detect_via_missing_size = detect_via_missing_size # Remove attributes that are None to keep the object clean self._remove_none_attributes() @@ -44,6 +47,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.""" @@ -86,6 +93,7 @@ def _set_job_defaults(self): ) self.remove_metadata_missing = JobParams( max_strikes=self.job_defaults.max_strikes, + detect_via_missing_size=False, ) self.remove_missing_files = JobParams() self.remove_orphans = JobParams() @@ -119,14 +127,14 @@ def _set_job_settings(self, job_name, job_config): ): # this triggers only when reading from yaml-file. for docker-compose, empty configs are not loaded, thus the entire job would not be parsed job.enabled = True elif isinstance(job_config, bool): - if job: + if job is not None: job.enabled = job_config else: job = JobParams(enabled=job_config) elif isinstance(job_config, dict): job_config.setdefault("enabled", True) - if job: + if job is not None: for key, value in job_config.items(): setattr(job, key, value) else: diff --git a/src/settings/_user_config.py b/src/settings/_user_config.py index 6cc4ac83..a1583b20 100644 --- a/src/settings/_user_config.py +++ b/src/settings/_user_config.py @@ -13,6 +13,7 @@ "LOG_LEVEL", "TEST_RUN", "TIMER", + "REQUEST_TIMEOUT", "SSL_VERIFICATION", "IGNORED_DOWNLOAD_CLIENTS", "PRIVATE_TRACKER_HANDLING", @@ -40,7 +41,7 @@ "SEARCH_UNMET_CUTOFF", "SEARCH_MISSING", ], - "instances": ["SONARR", "RADARR", "READARR", "LIDARR", "WHISPARR"], + "instances": ["SONARR", "SPORTARR", "RADARR", "READARR", "LIDARR", "WHISPARR"], "download_clients": ["QBITTORRENT"], } @@ -92,7 +93,11 @@ def _load_from_env() -> dict: parsed_value = _lowercase(parsed_value) except yaml.YAMLError as e: logger.error( - f"Failed to parse environment variable {key} as YAML:\n{e}", + "Could not parse %s as YAML. Check for smart quotes or " + "incorrect indentation. Configuration from this variable " + "was ignored.\n%s", + key, + e, ) parsed_value = {} section_config[key.lower()] = parsed_value diff --git a/src/utils/common.py b/src/utils/common.py index 62be171f..e09e195d 100644 --- a/src/utils/common.py +++ b/src/utils/common.py @@ -26,7 +26,14 @@ def sanitize_kwargs(data): for key, value in data.items(): if ( key.lower() - in {"username", "password", "x-api-key", "apikey", "cookies"} + in { + "username", + "password", + "x-api-key", + "apikey", + "cookies", + "authorization", + } and value ): redacted[key] = "[**redacted**]" @@ -42,7 +49,7 @@ async def make_request( method: str, endpoint: str, settings, - timeout: int = 15, + timeout: float | None = None, *, log_error=True, **kwargs, @@ -51,6 +58,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 +86,7 @@ async def make_request( endpoint, **kwargs, verify=settings.general.ssl_verification, - timeout=timeout, + timeout=request_timeout, ) response.raise_for_status() return response @@ -95,6 +107,18 @@ def wait_and_exit(seconds=30): sys.exit() +def is_definitive_setup_error(exc): + """Definitive = a configuration error that cannot heal without user action.""" + for candidate in (exc, exc.__cause__): + if getattr(candidate, "definitive", False): + return True + if isinstance(candidate, requests.exceptions.HTTPError): + response = getattr(candidate, "response", None) + if response is not None and response.status_code in (401, 403): + return True + return False + + def extract_json_from_response(response, key: str | None = None): try: data = response.json() diff --git a/src/utils/queue_manager.py b/src/utils/queue_manager.py index e05f1335..6e066f6c 100644 --- a/src/utils/queue_manager.py +++ b/src/utils/queue_manager.py @@ -38,7 +38,11 @@ async def get_queue_items(self, queue_scope): logger.debug( f"queue_manager.py/get_queue_items (normal) to determine orphans: Current queue ({len(normal_queue)} items) = {self.format_queue(normal_queue)}" ) - queue_items = [fq for fq in full_queue if fq not in normal_queue] + # Compare by queue item id, not by whole record: the two fetches are separate + # requests after a RefreshMonitoredDownloads, so sizeleft/timeleft of an active + # download differ between them and a record comparison marks it as an orphan (#322) + normal_ids = {nq.get("id") for nq in normal_queue} + queue_items = [fq for fq in full_queue if fq.get("id") not in normal_ids] elif queue_scope == "full": queue_items = await self._get_queue(full_queue=True) else: @@ -75,6 +79,7 @@ async def _refresh_queue(self): method="POST", endpoint=f"{self.arr.api_url}/command", settings=self.settings, + timeout=self.arr.timeout, json={"name": "RefreshMonitoredDownloads"}, headers={"X-Api-Key": self.arr.api_key}, ) @@ -109,6 +114,7 @@ async def fetch_queue_field(self, params, key: str | None = None): method="GET", endpoint=f"{self.arr.api_url}/queue", settings=self.settings, + timeout=self.arr.timeout, params=params, headers={"X-Api-Key": self.arr.api_key}, ) @@ -241,3 +247,21 @@ def filter_queue( filtered_items.append(item) break return filtered_items + + @staticmethod + def filter_missing_size(queue: list[dict]) -> list[dict]: + """ + Return queued items whose size is not yet known (no metadata fetched). + + Unlike qBittorrent, clients such as Transmission or Deluge do not surface a + "downloading metadata" message in the *arr queue, so a torrent stuck fetching + metadata cannot be matched by message. Such items are reported by the *arr as + status "queued" with size 0. This client-agnostic check catches them (see #57). + + The "size" key must be present so that incomplete items are not matched. + """ + return [ + item + for item in queue + if item.get("status") == "queued" and "size" in item and not item["size"] + ] diff --git a/src/utils/startup.py b/src/utils/startup.py index be4ed918..e61eef85 100644 --- a/src/utils/startup.py +++ b/src/utils/startup.py @@ -1,8 +1,24 @@ import warnings +from src.utils.common import wait_and_exit from src.utils.log_setup import logger +def _all_units(settings): + """All startup-checked units: download clients and arr instances.""" + return [ + *settings.download_clients.qbittorrent, + *settings.download_clients.sabnzbd, + *settings.instances, + ] + + +def _unit_label(unit): + """Printable identity; falls back to arr_type while name is not yet known.""" + name = getattr(unit, "name", None) or getattr(unit, "arr_type", "") + return f"{name} ({unit.base_url})" + + def show_welcome(settings): messages = [ "πŸŽ‰πŸŽ‰πŸŽ‰ Decluttarr - Application Started! πŸŽ‰πŸŽ‰πŸŽ‰", @@ -78,3 +94,56 @@ async def launch_steps(settings): settings.instances.check_any_arrs() for arr in settings.instances: await arr.setup() + + _exit_if_all_failed_definitively(settings) + + +def _exit_if_all_failed_definitively(settings): + """Exit only if every configured unit failed with a non-recoverable config error. + + A unit that failed transiently (timeout, connection) keeps the app alive: + it is retried every cycle and rejoins once its server responds. + """ + units = _all_units(settings) + if units and all( + not unit.ready and unit.failure_kind == "definitive" for unit in units + ): + logger.error( + "All configured instances and download clients failed their startup " + "checks with configuration errors that cannot self-heal. " + "Please review the errors above, fix your configuration, and restart.", + ) + wait_and_exit() + + +async def retry_degraded_instances(settings, watch_manager=None): + """Re-attempt setup of degraded units each cycle; report definitive failures.""" + for unit in _all_units(settings): + if unit.ready: + continue + if unit.failure_kind == "definitive": + logger.error( + f"SKIP | {_unit_label(unit)}: configuration error - {unit.last_error}. " + f"Fix configuration and restart. {unit.setup_tip}".rstrip(), + ) + continue + if await unit.setup(): + logger.info( + f"REJOINED | {_unit_label(unit)}: setup succeeded - resuming jobs" + ) + if ( + watch_manager + and getattr(unit, "arr_type", None) in ("sonarr", "sportarr", "radarr") + and settings.jobs.detect_deletions.enabled + ): + await watch_manager.setup_for_arr(unit) + else: + logger.warning( + f"SKIP | {_unit_label(unit)}: not reachable - {unit.last_error} " + f"(will retry next cycle)", + ) + + # A transient unit may have turned definitive on retry (e.g. a slow qBit + # finally answered, revealing a bad key). Re-evaluate so the app doesn't spin + # forever with nothing left that can heal. + _exit_if_all_failed_definitively(settings) diff --git a/src/utils/wanted_manager.py b/src/utils/wanted_manager.py index 2c363389..476b7190 100644 --- a/src/utils/wanted_manager.py +++ b/src/utils/wanted_manager.py @@ -42,6 +42,7 @@ async def fetch_wanted_field( method="GET", endpoint=f"{self.arr.api_url}/wanted/{missing_or_cutoff}", settings=self.settings, + timeout=self.arr.timeout, params=params, headers={"X-Api-Key": self.arr.api_key}, ) @@ -60,6 +61,7 @@ async def search_items(self, detail_ids): method="POST", endpoint=f"{self.arr.api_url}/command", settings=self.settings, + timeout=self.arr.timeout, json=json, headers={"X-Api-Key": self.arr.api_key}, ) diff --git a/tests/deletion_handler/test_deletion_handler.py b/tests/deletion_handler/test_deletion_handler.py index 8976e6d9..b0ad7fe2 100644 --- a/tests/deletion_handler/test_deletion_handler.py +++ b/tests/deletion_handler/test_deletion_handler.py @@ -165,6 +165,7 @@ def __init__(self): self.name = "Test" self.arr_type = "sonarr" self.base_url = "http://localhost" + self.ready = True self.called_paths = [] self.refreshed_ids = [] @@ -210,3 +211,20 @@ async def refresh_item(self, item_id): finally: watcher.stop() + + +@pytest.mark.asyncio +async def test_get_folders_to_watch_skips_not_ready_arr(): + arr_mock = MagicMock() + arr_mock.arr_type = "sonarr" + arr_mock.ready = False + arr_mock.get_root_folders = AsyncMock() + + settings = MagicMock() + settings.instances = [arr_mock] + watcher_manager = WatcherManager(settings) + + folders = await watcher_manager.get_folders_to_watch() + + assert folders == [] + arr_mock.get_root_folders.assert_not_awaited() diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py new file mode 100644 index 00000000..e7dfbfb4 --- /dev/null +++ b/tests/jobs/test_job_manager.py @@ -0,0 +1,215 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +import requests + +from src.job_manager import JobManager +from src.jobs.removal_handler import RemovalHandler +from src.settings._instances import Tracker +from src.utils.queue_manager import QueueManager + + +@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 + + +def _job_manager_with_deleted(deleted): + manager = JobManager(MagicMock()) + manager.arr = MagicMock() + manager.arr.tracker = Tracker() + manager.arr.tracker.deleted.extend(deleted) + return manager + + +def _queue_item(download_id, queue_id=1): + return { + "id": queue_id, + "downloadId": download_id, + "title": "Some.Release", + "protocol": "torrent", + "downloadClient": "qBittorrent", + } + + +@pytest.mark.asyncio +async def test_queue_has_items_forgets_deleted_downloads_that_are_back_in_queue(): + """A removal that did not stick must not block all future removals. + + The deleted tracker is otherwise only cleared once the queue is empty, so a + download that keeps coming back would be silently skipped by RemovalHandler + forever (strikes keep counting up, but no removal is ever triggered). + """ + # Arrange + manager = _job_manager_with_deleted(["still-in-queue", "actually-removed"]) + + get_queue_items = AsyncMock(return_value=[_queue_item("still-in-queue")]) + + # Act + with patch.object(QueueManager, "get_queue_items", get_queue_items): + has_items = await manager._queue_has_items() # pylint: disable=W0212 + + # Assert + assert has_items is True + assert manager.arr.tracker.deleted == ["actually-removed"] + # The orphans job that triggered this bug only shows up in the full queue + get_queue_items.assert_awaited_once_with("full") + + +@pytest.mark.asyncio +async def test_forgotten_download_is_removed_again_on_the_next_run(): + """After being forgotten, the download is removed instead of silently skipped.""" + # Arrange + manager = _job_manager_with_deleted(["comes-back"]) + manager.settings.general.public_tracker_handling = "remove" + manager.settings.download_clients.qbittorrent = [] + manager.settings.download_clients.get_download_client_by_name.return_value = ( + None, + None, + ) + manager.arr.remove_queue_item = AsyncMock() + queue_item = _queue_item("comes-back", queue_id=42) + + # Act + with patch.object( + QueueManager, "get_queue_items", AsyncMock(return_value=[queue_item]) + ): + await manager._queue_has_items() # pylint: disable=W0212 + await RemovalHandler( + arr=manager.arr, + settings=manager.settings, + job_name="remove_metadata_missing", + ).remove_downloads( + {"comes-back": {**queue_item, "queue_ids": [queue_item["id"]]}}, + blocklist=True, + ) + + # Assert + manager.arr.remove_queue_item.assert_awaited_once_with( + queue_id=42, blocklist=True, remove_from_client=True + ) + + +@pytest.mark.asyncio +async def test_queue_has_items_keeps_deleted_downloads_that_are_gone(): + """Downloads that really were removed stay in the tracker (no double removal).""" + # Arrange + manager = _job_manager_with_deleted(["actually-removed"]) + + # Act + with patch.object( + QueueManager, + "get_queue_items", + AsyncMock(return_value=[_queue_item("something-else")]), + ): + has_items = await manager._queue_has_items() # pylint: disable=W0212 + + # Assert + assert has_items is True + assert manager.arr.tracker.deleted == ["actually-removed"] + + +@pytest.mark.asyncio +async def test_obsolete_tagged_download_is_not_tagged_again_on_the_next_run(): + """Obsolete-tagged downloads stay in the queue by design and must not be re-tagged. + + With private_tracker_handling/public_tracker_handling set to "obsolete_tag", + the download is tagged instead of removed and deliberately remains in the + queue, so its presence is not evidence of a failed removal. + """ + # Arrange + manager = _job_manager_with_deleted([]) + manager.settings.general.public_tracker_handling = "obsolete_tag" + manager.settings.general.obsolete_tag = "Obsolete" + qbit = MagicMock(ready=True, set_tag=AsyncMock()) + manager.settings.download_clients.qbittorrent = [qbit] + manager.settings.download_clients.get_download_client_by_name.return_value = ( + qbit, + "qbittorrent", + ) + queue_item = _queue_item("tagged", queue_id=7) + + # Act + for _ in range(2): + with patch.object( + QueueManager, "get_queue_items", AsyncMock(return_value=[queue_item]) + ): + await manager._queue_has_items() # pylint: disable=W0212 + await RemovalHandler( + arr=manager.arr, + settings=manager.settings, + job_name="remove_stalled", + ).remove_downloads( + {"tagged": {**queue_item, "queue_ids": [queue_item["id"]]}}, + blocklist=False, + ) + + # Assert + qbit.set_tag.assert_awaited_once_with(tags=["Obsolete"], hashes=["tagged"]) + manager.arr.remove_queue_item.assert_not_called() diff --git a/tests/jobs/test_removal_handler.py b/tests/jobs/test_removal_handler.py index 08a754c1..a9dc33ab 100644 --- a/tests/jobs/test_removal_handler.py +++ b/tests/jobs/test_removal_handler.py @@ -52,3 +52,87 @@ async def test_get_handling_method( "A", affected_download ) assert result == expected + + +@pytest.mark.asyncio +async def test_tag_as_obsolete_skips_not_ready_qbit(): + """A degraded qbit must not have set_tag called during obsolete-tagging.""" + ready_qbit = AsyncMock() + ready_qbit.ready = True + degraded_qbit = AsyncMock() + degraded_qbit.ready = False + + arr = AsyncMock() + settings = MagicMock() + settings.download_clients.qbittorrent = [ready_qbit, degraded_qbit] + settings.general.obsolete_tag = "Obsolete" + + handler = RemovalHandler(arr=arr, settings=settings, job_name="remove_stalled") + await handler._tag_as_obsolete({"title": "Some.Release"}, download_id="hash1") + + ready_qbit.set_tag.assert_awaited_once() + degraded_qbit.set_tag.assert_not_awaited() + + +def _handler_for(handling_method): + """RemovalHandler wired so every download resolves to the given handling method.""" + arr = AsyncMock() + arr.tracker.deleted = [] + arr.tracker.private = ["hash1"] + + settings = MagicMock() + settings.download_clients.qbittorrent = [AsyncMock(ready=True)] + settings.download_clients.get_download_client_by_name.return_value = ( + "qbt", + "qbittorrent", + ) + settings.general.private_tracker_handling = handling_method + settings.general.public_tracker_handling = handling_method + + handler = RemovalHandler(arr=arr, settings=settings, job_name="remove_stalled") + return handler, arr, settings + + +def _affected(title="Some.Release"): + return { + "hash1": { + "downloadClient": "qbt", + "protocol": "torrent", + "queue_ids": [42], + "title": title, + } + } + + +@pytest.mark.asyncio +async def test_remove_from_queue_keeps_the_torrent_in_the_client(): + """remove_from_queue clears the arr queue entry but must not delete the download.""" + handler, arr, _ = _handler_for("remove_from_queue") + + await handler.remove_downloads(_affected(), blocklist=True) + + arr.remove_queue_item.assert_awaited_once_with( + queue_id=42, blocklist=True, remove_from_client=False + ) + + +@pytest.mark.asyncio +async def test_remove_from_queue_does_not_tag_the_torrent(): + """Unlike obsolete_tag, this mode acts on the arr side only.""" + handler, _, settings = _handler_for("remove_from_queue") + + await handler.remove_downloads(_affected(), blocklist=True) + + settings.download_clients.qbittorrent[0].set_tag.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_remove_still_deletes_from_the_client(): + """The existing remove mode is unchanged.""" + handler, arr, _ = _handler_for("remove") + + await handler.remove_downloads(_affected(), blocklist=True) + + assert ( + arr.remove_queue_item.await_args.kwargs.get("remove_from_client", True) is True + ) diff --git a/tests/jobs/test_removal_job.py b/tests/jobs/test_removal_job.py new file mode 100644 index 00000000..7297fb19 --- /dev/null +++ b/tests/jobs/test_removal_job.py @@ -0,0 +1,55 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock + +from src.jobs.remove_stalled import RemoveStalled +from src.utils.queue_manager import QueueManager + + +def _job_with_clients(): + """A RemovalJob (via a concrete subclass) whose settings resolve clients by name.""" + job = RemoveStalled.__new__(RemoveStalled) + job.job_name = "remove_stalled" + job.arr = MagicMock() + job.arr.name = "Sonarr" + + ready = SimpleNamespace(name="qbit-ready", ready=True) + degraded = SimpleNamespace(name="qbit-degraded", ready=False) + + def resolver(name, *args, **kwargs): + return { + "qbit-ready": (ready, "qbittorrent"), + "qbit-degraded": (degraded, "qbittorrent"), + }.get(name, (None, None)) + + job.settings = MagicMock() + job.settings.download_clients.get_download_client_by_name.side_effect = resolver + return job + + +def test_ignore_degraded_client_downloads_is_fail_closed(): + """Downloads on a degraded client are left untouched; healthy and + unconfigured clients are unaffected (this is the guard that prevents the + data-loss scenario where a degraded qBit's protected torrents get deleted). + + affected_downloads is built via the real group_by_download_id so the grouped + dict shape is exercised (a list-shaped fixture would hide a KeyError).""" + job = _job_with_clients() + grouped = QueueManager.group_by_download_id( + None, + [ + {"downloadId": "on_ready", "id": 1, "downloadClient": "qbit-ready"}, + {"downloadId": "on_degraded", "id": 2, "downloadClient": "qbit-degraded"}, + { + "downloadId": "on_unconfigured", + "id": 3, + "downloadClient": "other-client", + }, + ], + ) + job.affected_downloads = grouped + + job._ignore_degraded_client_downloads() + + assert "on_ready" in job.affected_downloads # healthy -> still removable + assert "on_degraded" not in job.affected_downloads # degraded -> protected + assert "on_unconfigured" in job.affected_downloads # unconfigured -> unchanged diff --git a/tests/jobs/test_remove_metadata_missing.py b/tests/jobs/test_remove_metadata_missing.py index 7f92c37b..ff7bdb48 100644 --- a/tests/jobs/test_remove_metadata_missing.py +++ b/tests/jobs/test_remove_metadata_missing.py @@ -1,9 +1,73 @@ +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + import pytest from src.jobs.remove_metadata_missing import RemoveMetadataMissing from tests.jobs.utils import shared_fix_affected_items, shared_test_affected_items +def _metadata_missing_job( + *, + max_strikes=None, + protected=(), + ignored_download_clients=(), + detect_via_missing_size=False, +): + arr = MagicMock() + arr.name = "Sonarr" + arr.detail_item_id_key = "episodeId" + arr.tracker = SimpleNamespace( + protected=list(protected), + private=[], + deleted=[], + defective={}, + ) + arr.remove_queue_item = AsyncMock() + + job = SimpleNamespace(enabled=True, detect_via_missing_size=detect_via_missing_size) + if max_strikes is not None: + job.max_strikes = max_strikes + + settings = MagicMock() + settings.jobs.remove_metadata_missing = job + settings.general.ignored_download_clients = list(ignored_download_clients) + settings.download_clients.qbittorrent = [] + settings.download_clients.get_download_client_by_name.return_value = (None, None) + + return RemoveMetadataMissing(arr, settings, "remove_metadata_missing") + + +def _metadata_queue_item(download_id="metadata-stuck", queue_id=101, **changes): + item = { + "id": queue_id, + "downloadId": download_id, + "title": "Metadata.Stuck.Release", + "protocol": "torrent", + "downloadClient": "qBittorrent", + "status": "queued", + "errorMessage": "qBittorrent is downloading metadata", + "size": 0, + "episodeId": None, + "seriesId": None, + } + item.update(changes) + return item + + +def _serve_api_queues(removal_job, *, normal, full): + queues = {False: normal, True: full} + removal_job.queue_manager._refresh_queue = AsyncMock() + removal_job.queue_manager._get_total_records_count = AsyncMock( + side_effect=lambda full_queue: len(queues[full_queue]) + ) + removal_job.queue_manager._get_arr_records = AsyncMock( + side_effect=lambda full_queue, _total_records_count: [ + dict(item) for item in queues[full_queue] + ] + ) + + # Test to check if items with the specific error message are included in affected items with parameterized data @pytest.mark.asyncio @pytest.mark.parametrize( @@ -88,6 +152,277 @@ async def test_find_affected_items(queue_data, expected_download_ids): # Arrange removal_job = shared_fix_affected_items(RemoveMetadataMissing, queue_data) + removal_job.job = SimpleNamespace(detect_via_missing_size=False) # Act and Assert await shared_test_affected_items(removal_job, expected_download_ids) + + +# Tests the opt-in, client-agnostic detection of items stuck without metadata +# (status "queued" + size 0), e.g. on Transmission/Deluge which do not surface the +# qBittorrent "downloading metadata" message in the *arr queue (see issue #57). +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("detect_via_missing_size", "queue_data", "expected_download_ids"), + [ + # Disabled (default): a queued size-0 item is NOT flagged; only the qBit message is. + ( + False, + [ + { + "id": 1, + "downloadId": "a", + "status": "queued", + "size": 0, + "errorMessage": None, + }, + { + "id": 2, + "downloadId": "b", + "status": "queued", + "errorMessage": "qBittorrent is downloading metadata", + }, + ], + ["b"], + ), + # Enabled: queued + size 0 is flagged; size > 0 and non-queued are left alone. + ( + True, + [ + { + "id": 1, + "downloadId": "a", + "status": "queued", + "size": 0, + "errorMessage": None, + }, + { + "id": 2, + "downloadId": "b", + "status": "queued", + "size": 1234, + "errorMessage": None, + }, + { + "id": 3, + "downloadId": "c", + "status": "downloading", + "size": 0, + "errorMessage": None, + }, + ], + ["a"], + ), + # Enabled: an item matching BOTH the qBit message and size 0 is not duplicated. + ( + True, + [ + { + "id": 1, + "downloadId": "a", + "status": "queued", + "size": 0, + "errorMessage": "qBittorrent is downloading metadata", + }, + { + "id": 2, + "downloadId": "b", + "status": "queued", + "size": 0, + "errorMessage": None, + }, + ], + ["a", "b"], + ), + # Enabled but no size key present (e.g. partial item): not matched. + ( + True, + [ + {"id": 1, "downloadId": "a", "status": "queued", "errorMessage": None}, + ], + [], + ), + ], +) +async def test_find_affected_items_via_missing_size( + detect_via_missing_size, queue_data, expected_download_ids +): + # Arrange + removal_job = shared_fix_affected_items(RemoveMetadataMissing, queue_data) + removal_job.job = MagicMock(detect_via_missing_size=detect_via_missing_size) + removal_job.queue_manager.get_queue_items = AsyncMock( + return_value=[dict(item) for item in queue_data] + ) + + # Act and Assert + await shared_test_affected_items(removal_job, expected_download_ids) + + +@pytest.mark.asyncio +async def test_run_detects_metadata_stuck_item_present_only_in_full_queue(): + removal_job = _metadata_missing_job() + full_only_item = _metadata_queue_item() + queues = {"normal": [], "full": [full_only_item]} + removal_job.queue_manager.get_queue_items = AsyncMock( + side_effect=lambda queue_scope: queues[queue_scope] + ) + + removed = await removal_job.run() + + assert removed == 1 + removal_job.queue_manager.get_queue_items.assert_awaited_once_with( + queue_scope="full" + ) + removal_job.arr.remove_queue_item.assert_awaited_once_with( + queue_id=full_only_item["id"], blocklist=True, remove_from_client=True + ) + + +@pytest.mark.asyncio +async def test_run_still_detects_metadata_stuck_item_in_normal_queue(): + removal_job = _metadata_missing_job() + normal_item = _metadata_queue_item( + download_id="normal-metadata-stuck", + queue_id=201, + episodeId=42, + seriesId=7, + ) + _serve_api_queues(removal_job, normal=[normal_item], full=[normal_item]) + + removed = await removal_job.run() + + assert removed == 1 + removal_job.arr.remove_queue_item.assert_awaited_once_with( + queue_id=normal_item["id"], blocklist=True, remove_from_client=True + ) + + +@pytest.mark.asyncio +async def test_run_ignores_full_only_items_without_exact_status_and_error(): + removal_job = _metadata_missing_job() + full_only_items = [ + _metadata_queue_item( + download_id="wrong-status", queue_id=211, status="downloading" + ), + _metadata_queue_item( + download_id="wrong-error", + queue_id=212, + errorMessage="Some other error", + ), + _metadata_queue_item( + download_id="size-zero-only", queue_id=213, errorMessage=None + ), + ] + _serve_api_queues(removal_job, normal=[], full=full_only_items) + + removed = await removal_job.run() + + assert removed == 0 + removal_job.arr.remove_queue_item.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_run_keeps_ignored_client_and_protected_tag_downloads(): + removal_job = _metadata_missing_job( + protected=["protected-download"], + ignored_download_clients=["Ignored qBittorrent"], + ) + ignored_item = _metadata_queue_item( + download_id="ignored-download", + queue_id=221, + downloadClient="Ignored qBittorrent", + ) + protected_item = _metadata_queue_item( + download_id="protected-download", queue_id=222 + ) + removable_item = _metadata_queue_item( + download_id="removable-download", queue_id=223 + ) + _serve_api_queues( + removal_job, + normal=[], + full=[ignored_item, protected_item, removable_item], + ) + + removed = await removal_job.run() + + assert removed == 1 + removal_job.arr.remove_queue_item.assert_awaited_once_with( + queue_id=removable_item["id"], blocklist=True, remove_from_client=True + ) + assert removal_job.arr.tracker.deleted == [removable_item["downloadId"]] + + +@pytest.mark.asyncio +async def test_run_preserves_strikes_and_blocklist_for_full_only_item(): + removal_job = _metadata_missing_job(max_strikes=1) + full_only_item = _metadata_queue_item(download_id="struck-download", queue_id=231) + _serve_api_queues(removal_job, normal=[], full=[full_only_item]) + + first_removed = await removal_job.run() + + assert first_removed == 0 + removal_job.arr.remove_queue_item.assert_not_awaited() + assert ( + removal_job.arr.tracker.defective["remove_metadata_missing"][ + full_only_item["downloadId"] + ]["strikes"] + == 1 + ) + + second_removed = await removal_job.run() + + assert second_removed == 1 + assert ( + removal_job.arr.tracker.defective["remove_metadata_missing"][ + full_only_item["downloadId"] + ]["strikes"] + == 2 + ) + removal_job.arr.remove_queue_item.assert_awaited_once_with( + queue_id=full_only_item["id"], blocklist=True, remove_from_client=True + ) + + +@pytest.mark.asyncio +async def test_run_removes_duplicate_queue_rows_only_once(): + removal_job = _metadata_missing_job() + first_row = _metadata_queue_item(download_id="duplicate-download", queue_id=241) + second_row = _metadata_queue_item(download_id="duplicate-download", queue_id=242) + _serve_api_queues(removal_job, normal=[], full=[first_row, second_row]) + + removed = await removal_job.run() + + assert removed == 1 + removal_job.arr.remove_queue_item.assert_awaited_once_with( + queue_id=first_row["id"], blocklist=True, remove_from_client=True + ) + assert removal_job.arr.tracker.deleted == [first_row["downloadId"]] + + +@pytest.mark.asyncio +async def test_missing_size_opt_in_remains_limited_to_normal_queue(): + removal_job = _metadata_missing_job(detect_via_missing_size=True) + full_only_size_zero = _metadata_queue_item( + download_id="full-only-size-zero", queue_id=251, errorMessage=None + ) + normal_size_zero = _metadata_queue_item( + download_id="normal-size-zero", + queue_id=252, + errorMessage=None, + episodeId=52, + seriesId=8, + ) + _serve_api_queues( + removal_job, + normal=[normal_size_zero], + full=[full_only_size_zero, normal_size_zero], + ) + + removed = await removal_job.run() + + assert removed == 1 + removal_job.arr.remove_queue_item.assert_awaited_once_with( + queue_id=normal_size_zero["id"], blocklist=True, remove_from_client=True + ) + assert removal_job.arr.tracker.deleted == [normal_size_zero["downloadId"]] diff --git a/tests/jobs/test_remove_slow.py b/tests/jobs/test_remove_slow.py index 35637135..0a0dce2c 100644 --- a/tests/jobs/test_remove_slow.py +++ b/tests/jobs/test_remove_slow.py @@ -391,3 +391,45 @@ async def test_find_affected_items_simple(queue_item, should_be_affected): assert ( not affected_items ), f"Item {queue_item.get('downloadId')} should NOT be affected" + + +@pytest.mark.asyncio +async def test_add_download_client_resolves_ready_only(): + """A not-ready client resolves to None so downstream job logic skips it.""" + job = RemoveSlow.__new__(RemoveSlow) + job.settings = MagicMock() + job.settings.download_clients.get_download_client_by_name.return_value = ( + None, + None, + ) + job.queue = [{"downloadClient": "qBittorrent"}] + + await job.add_download_client_to_queue_items() + + job.settings.download_clients.get_download_client_by_name.assert_called_once_with( + "qBittorrent", ready_only=True + ) + assert job.queue[0]["download_client"] is None + assert job.queue[0]["download_client_type"] is None + + +@pytest.mark.asyncio +async def test_update_bandwidth_usage_skips_degraded_client(): + """A degraded client (mapped to None) must not have set_bandwidth_usage called.""" + job = RemoveSlow.__new__(RemoveSlow) + job.queue = [{"download_client": None, "download_client_type": None}] + + # Should be a no-op β€” no client to call, no exception. + await job.update_bandwidth_usage() + + +@pytest.mark.asyncio +async def test_update_bandwidth_usage_calls_ready_qbit(): + """A ready qbit client still gets its bandwidth refreshed (regression).""" + client = AsyncMock() + job = RemoveSlow.__new__(RemoveSlow) + job.queue = [{"download_client": client, "download_client_type": "qbittorrent"}] + + await job.update_bandwidth_usage() + + client.set_bandwidth_usage.assert_awaited_once() diff --git a/tests/jobs/test_remove_unmonitored.py b/tests/jobs/test_remove_unmonitored.py index a3191397..ca6aec92 100644 --- a/tests/jobs/test_remove_unmonitored.py +++ b/tests/jobs/test_remove_unmonitored.py @@ -1,6 +1,8 @@ -from unittest.mock import AsyncMock +import logging +from unittest.mock import AsyncMock, MagicMock import pytest +import requests from src.jobs.remove_unmonitored import RemoveUnmonitored from tests.jobs.utils import shared_fix_affected_items, shared_test_affected_items @@ -73,3 +75,37 @@ async def test_find_affected_items(queue_data, monitored_ids, expected_download_ removal_job.arr.is_monitored = AsyncMock(side_effect=lambda id_: monitored_ids[id_]) # Act and Assert await shared_test_affected_items(removal_job, expected_download_ids) + + +@pytest.mark.asyncio +async def test_stale_queue_item_is_skipped_and_processing_continues(caplog): + queue_data = [ + {"downloadId": "stale", "detail_item_id": 101}, + {"downloadId": "unmonitored", "detail_item_id": 102}, + ] + removal_job = shared_fix_affected_items(RemoveUnmonitored, queue_data) + removal_job.arr.detail_item_key = "book" + removal_job.arr.name = "Readarr" + + not_found = requests.exceptions.HTTPError("404 Not Found") + not_found.response = MagicMock(status_code=404) + removal_job.arr.is_monitored = AsyncMock(side_effect=[not_found, False]) + + with caplog.at_level(logging.WARNING, logger="src.utils.log_setup"): + await shared_test_affected_items(removal_job, ["unmonitored"]) + + removal_job.arr.is_monitored.assert_awaited() + assert "Skipping stale queue item stale" in caplog.text + assert "book 101 no longer exists on Readarr" in caplog.text + + +@pytest.mark.asyncio +async def test_non_404_monitoring_error_is_raised(): + queue_data = [{"downloadId": "failed", "detail_item_id": 101}] + removal_job = shared_fix_affected_items(RemoveUnmonitored, queue_data) + server_error = requests.exceptions.HTTPError("500 Server Error") + server_error.response = MagicMock(status_code=500) + removal_job.arr.is_monitored = AsyncMock(side_effect=server_error) + + with pytest.raises(requests.exceptions.HTTPError, match="500 Server Error"): + await removal_job._find_affected_items() # pylint: disable=protected-access diff --git a/tests/settings/test_download_clients.py b/tests/settings/test_download_clients.py new file mode 100644 index 00000000..e51a77ac --- /dev/null +++ b/tests/settings/test_download_clients.py @@ -0,0 +1,46 @@ +from types import SimpleNamespace + +from src.settings._download_clients import DownloadClients + + +def _clients(qbits): + """A DownloadClients instance with the given qbit list (bypassing __init__).""" + dc = DownloadClients.__new__(DownloadClients) + dc.qbittorrent = qbits + dc.sabnzbd = [] + return dc + + +def test_ready_only_returns_ready_client(): + client = SimpleNamespace(name="qBittorrent", ready=True) + dc = _clients([client]) + assert dc.get_download_client_by_name("qBittorrent", ready_only=True) == ( + client, + "qbittorrent", + ) + + +def test_ready_only_skips_not_ready_client(): + client = SimpleNamespace(name="qBittorrent", ready=False) + dc = _clients([client]) + # A degraded client is treated as unavailable so jobs never call it. + assert dc.get_download_client_by_name("qBittorrent", ready_only=True) == ( + None, + None, + ) + + +def test_default_ignores_readiness(): + # Backward compatibility: without ready_only, readiness is not considered. + client = SimpleNamespace(name="qBittorrent", ready=False) + dc = _clients([client]) + assert dc.get_download_client_by_name("qBittorrent") == (client, "qbittorrent") + + +def test_unknown_name_returns_none(): + client = SimpleNamespace(name="qBittorrent", ready=True) + dc = _clients([client]) + assert dc.get_download_client_by_name("does-not-exist", ready_only=True) == ( + None, + None, + ) diff --git a/tests/settings/test_download_clients_qbit.py b/tests/settings/test_download_clients_qbit.py index 06578006..8f51d3ff 100644 --- a/tests/settings/test_download_clients_qbit.py +++ b/tests/settings/test_download_clients_qbit.py @@ -1,6 +1,6 @@ import pytest - from requests.cookies import RequestsCookieJar + from src.settings._download_clients_qbit import QbitClient, QbitError @@ -38,3 +38,242 @@ def test_extract_sid_failures(cookies): with pytest.raises(QbitError, match="No qBit cookie found"): QbitClient.extract_sid(jar) + + +from unittest.mock import AsyncMock, MagicMock, patch + +import requests + + +@pytest.mark.asyncio +async def test_setup_reachability_timeout_marks_transient(): + settings = MagicMock() + client = QbitClient(settings, base_url="http://qbit:8080") + + with patch( + "src.settings._download_clients_qbit.make_request", + new_callable=AsyncMock, + side_effect=requests.exceptions.ReadTimeout("Read timed out."), + ): + result = await client.setup() + + assert result is False + assert client.ready is False + assert client.failure_kind == "transient" + assert "Read timed out." in client.last_error + + +@pytest.mark.asyncio +async def test_setup_old_version_marks_definitive(): + settings = MagicMock() + settings.min_versions.qbittorrent = "4.3.0" + client = QbitClient(settings, base_url="http://qbit:8080") + client.check_qbit_reachability = AsyncMock() + client.refresh_cookie = AsyncMock() + client.fetch_version = AsyncMock() + client.version = "3.0.0" + + await client.setup() + + assert client.ready is False + assert client.failure_kind == "definitive" + + +@pytest.mark.asyncio +async def test_setup_cookie_refresh_failure_marks_transient(): + settings = MagicMock() + client = QbitClient(settings, base_url="http://qbit:8080") + client.check_qbit_reachability = AsyncMock() + client.refresh_cookie = AsyncMock( + side_effect=QbitError(ConnectionError("Login failed.")) + ) + + await client.setup() + + assert client.ready is False + assert client.failure_kind == "transient" + + +@pytest.mark.asyncio +async def test_setup_bad_password_marks_definitive(): + """Wrong qBit password (HTTP 200 + 'Fails.') is definitive, not retried forever.""" + settings = MagicMock() + client = QbitClient( + settings, base_url="http://qbit:8080", username="u", password="wrong" + ) + resp = MagicMock(text="Fails.") + with patch( + "src.settings._download_clients_qbit.make_request", + new_callable=AsyncMock, + return_value=resp, + ): + await client.setup() + + assert client.ready is False + assert client.failure_kind == "definitive" + + +# --- API-key (qBit 5.2+) authentication --- + +VALID_KEY = "qbt_" + "a" * 28 + + +def test_auth_kwargs_key_mode_uses_bearer_header(): + client = QbitClient(MagicMock(), base_url="http://qbit:8080", api_key=VALID_KEY) + assert client._auth_kwargs() == { + "headers": {"Authorization": f"Bearer {VALID_KEY}"} + } + + +def test_auth_kwargs_password_mode_uses_cookie(): + client = QbitClient( + MagicMock(), base_url="http://qbit:8080", username="u", password="p" + ) + client.cookie = {"SID": "abc"} + assert client._auth_kwargs() == {"cookies": {"SID": "abc"}} + + +def test_empty_api_key_falls_back_to_password_mode(): + client = QbitClient( + MagicMock(), + base_url="http://qbit:8080", + api_key=" ", + username="u", + password="p", + ) + client.cookie = {"SID": "abc"} + # Whitespace-only key is treated as unset. + assert client._auth_kwargs() == {"cookies": {"SID": "abc"}} + + +def test_both_provided_key_wins_and_logs(caplog): + with caplog.at_level("INFO"): + client = QbitClient( + MagicMock(), + base_url="http://qbit:8080", + api_key=VALID_KEY, + username="u", + password="p", + ) + assert "Authorization" in client._auth_kwargs()["headers"] + assert any("using api_key" in r.message for r in caplog.records) + + +def test_old_version_with_api_key_warns_without_raising(caplog): + client = QbitClient(MagicMock(), base_url="http://qbit:8080", api_key=VALID_KEY) + client.version = "5.1.2" + + with caplog.at_level("WARNING"): + client.log_auth_version_guidance() + + assert "does not support API-key authentication" in caplog.text + assert "API key did not authenticate this connection" in caplog.text + + +def test_supported_version_with_password_recommends_api_key(caplog): + client = QbitClient( + MagicMock(), base_url="http://qbit:8080", username="u", password="p" + ) + client.version = "5.2.0" + + with caplog.at_level("INFO"): + client.log_auth_version_guidance() + + assert "supports API-key authentication" in caplog.text + assert "replacing username/password with api_key" in caplog.text + + +@pytest.mark.parametrize( + ("version_value", "credentials"), + [ + ("5.2.0", {"api_key": VALID_KEY}), + ("5.1.2", {"username": "u", "password": "p"}), + ], +) +def test_auth_version_guidance_omits_irrelevant_advice( + caplog, version_value, credentials +): + client = QbitClient(MagicMock(), base_url="http://qbit:8080", **credentials) + client.version = version_value + + with caplog.at_level("INFO"): + client.log_auth_version_guidance() + + assert "API-key authentication" not in caplog.text + + +@pytest.mark.asyncio +async def test_fetch_version_key_mode_sends_bearer_not_cookie(): + client = QbitClient(MagicMock(), base_url="http://qbit:8080", api_key=VALID_KEY) + + with patch( + "src.settings._download_clients_qbit.make_request", new_callable=AsyncMock + ) as mock_req: + mock_req.return_value = MagicMock(text="_v5.2.0") + await client.fetch_version() + + kwargs = mock_req.call_args.kwargs + assert kwargs["headers"] == {"Authorization": f"Bearer {VALID_KEY}"} + assert "cookies" not in kwargs + + +@pytest.mark.asyncio +async def test_refresh_cookie_noop_in_key_mode(): + client = QbitClient(MagicMock(), base_url="http://qbit:8080", api_key=VALID_KEY) + + with patch( + "src.settings._download_clients_qbit.make_request", new_callable=AsyncMock + ) as mock_req: + await client.refresh_cookie() + + mock_req.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_reachability_key_mode_probes_app_version(): + client = QbitClient(MagicMock(), base_url="http://qbit:8080", api_key=VALID_KEY) + + with patch( + "src.settings._download_clients_qbit.make_request", new_callable=AsyncMock + ) as mock_req: + await client.check_qbit_reachability() + + method, endpoint = mock_req.call_args.args[0], mock_req.call_args.args[1] + assert method == "get" + assert endpoint.endswith("/app/version") + + +@pytest.mark.asyncio +async def test_reachability_password_mode_still_posts_login(): + client = QbitClient( + MagicMock(), base_url="http://qbit:8080", username="u", password="p" + ) + + with patch( + "src.settings._download_clients_qbit.make_request", new_callable=AsyncMock + ) as mock_req: + await client.check_qbit_reachability() + + method, endpoint = mock_req.call_args.args[0], mock_req.call_args.args[1] + assert method == "post" + assert endpoint.endswith("/auth/login") + + +@pytest.mark.asyncio +async def test_setup_bad_key_403_marks_definitive_with_tip(): + http_error = requests.exceptions.HTTPError("403 Forbidden") + http_error.response = MagicMock(status_code=403) + + client = QbitClient(MagicMock(), base_url="http://qbit:8080", api_key=VALID_KEY) + + with patch( + "src.settings._download_clients_qbit.make_request", + new_callable=AsyncMock, + side_effect=http_error, + ): + result = await client.setup() + + assert result is False + assert client.ready is False + assert client.failure_kind == "definitive" + assert "5.2" in client.setup_tip diff --git a/tests/settings/test_general.py b/tests/settings/test_general.py new file mode 100644 index 00000000..b7789eb7 --- /dev/null +++ b/tests/settings/test_general.py @@ -0,0 +1,20 @@ +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 + + +def test_remove_from_queue_is_a_valid_tracker_handling_value(): + """remove_from_queue clears the arr queue while leaving the torrent in the client.""" + general = General({"general": {"private_tracker_handling": "remove_from_queue"}}) + + assert general.private_tracker_handling == "remove_from_queue" diff --git a/tests/settings/test_instances.py b/tests/settings/test_instances.py index 17934992..493dea35 100644 --- a/tests/settings/test_instances.py +++ b/tests/settings/test_instances.py @@ -1,10 +1,34 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +import requests from src.settings._instances import ArrInstance +def _make_settings(min_version="4.0.0"): + settings = MagicMock() + settings.min_versions.sonarr = min_version + return settings + + +def _response(json_data): + response = MagicMock() + response.json = MagicMock(return_value=json_data) + return response + + +def _healthy_responses(app_name="Sonarr", app_version="9.9.9", ui_language=1): + """Responses for the three make_request calls in a full successful setup.""" + return [ + _response( + {"instanceName": "MockSonarr", "appName": app_name, "version": app_version} + ), + _response({"uiLanguage": ui_language}), + _response([]), # downloadclient list + ] + + @pytest.mark.asyncio @pytest.mark.parametrize( "arr_type, expected_key", @@ -39,6 +63,7 @@ async def test_get_refresh_item_calls_make_request_with_correct_params( "get", arr.api_url + "/" + expected_key, settings, + timeout=arr.timeout, headers={"X-Api-Key": api_key}, ) assert result == fake_json @@ -60,3 +85,157 @@ async def test_get_refresh_item_by_path_returns_correct_item(): mock_method.assert_awaited_once() assert result == {"id": 456, "path": "/media/folder2"} + + +@pytest.mark.asyncio +async def test_setup_timeout_marks_transient_and_does_not_exit(): + arr = ArrInstance(_make_settings(), "sonarr", "http://sonarr/", "test_key") + + with ( + patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=requests.exceptions.ReadTimeout("Read timed out."), + ), + patch("src.settings._instances.wait_and_exit") as mock_exit, + ): + result = await arr.setup() + + mock_exit.assert_not_called() + assert result is False + assert arr.ready is False + assert arr.failure_kind == "transient" + assert "Read timed out." in arr.last_error + + +@pytest.mark.asyncio +async def test_setup_401_marks_definitive(): + http_error = requests.exceptions.HTTPError("401 Client Error") + http_error.response = MagicMock(status_code=401) + + arr = ArrInstance(_make_settings(), "sonarr", "http://sonarr/", "test_key") + + with ( + patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=http_error, + ), + patch("src.settings._instances.wait_and_exit") as mock_exit, + ): + await arr.setup() + + mock_exit.assert_not_called() + assert arr.ready is False + assert arr.failure_kind == "definitive" + assert "API_KEY" in arr.setup_tip + + +@pytest.mark.asyncio +async def test_setup_non_english_ui_marks_definitive(): + arr = ArrInstance(_make_settings(), "sonarr", "http://sonarr/", "test_key") + + with patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=_healthy_responses(ui_language=2), + ): + await arr.setup() + + assert arr.ready is False + assert arr.failure_kind == "definitive" + + +@pytest.mark.asyncio +async def test_setup_recovers_on_retry(): + arr = ArrInstance(_make_settings(), "sonarr", "http://sonarr/", "test_key") + + with patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=requests.exceptions.ReadTimeout("Read timed out."), + ): + assert await arr.setup() is False + + with patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=_healthy_responses(), + ): + assert await arr.setup() is True + + assert arr.ready is True + assert arr.failure_kind is None + assert arr.last_error is None + assert arr.name == "MockSonarr" + + +@pytest.mark.asyncio +async def test_setup_same_error_logs_tip_block_once(caplog): + arr = ArrInstance(_make_settings(), "sonarr", "http://sonarr/", "test_key") + + with patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=requests.exceptions.ReadTimeout("Read timed out."), + ): + with caplog.at_level("ERROR"): + await arr.setup() + await arr.setup() + + tip_blocks = [r for r in caplog.records if "❗️" in r.message] + assert len(tip_blocks) == 1 + + +@pytest.mark.asyncio +async def test_setup_wrong_arr_type_and_old_version_still_pass(): + arr = ArrInstance(_make_settings(), "sonarr", "http://sonarr/", "test_key") + + with patch( + "src.settings._instances.make_request", + new_callable=AsyncMock, + side_effect=_healthy_responses(app_name="Radarr", app_version="0.0.1"), + ): + assert await arr.setup() is True + + assert arr.ready is True + + +def _arr_for_queue_removal(): + arr = ArrInstance.__new__(ArrInstance) + arr.api_url = "http://sonarr/api/v3" + arr.api_key = "test_key" + arr.settings = MagicMock() + arr._timeout = 15 + return arr + + +@pytest.mark.asyncio +async def test_remove_queue_item_deletes_from_client_by_default(): + arr = _arr_for_queue_removal() + + with patch( + "src.settings._instances.make_request", new=AsyncMock() + ) as mocked_request: + mocked_request.return_value = MagicMock(status_code=200) + await arr.remove_queue_item(queue_id=7, blocklist=True) + + assert mocked_request.call_args.kwargs["params"]["removeFromClient"] is True + + +@pytest.mark.asyncio +async def test_remove_queue_item_can_leave_the_torrent_in_the_client(): + """remove_from_client=False clears the queue entry but keeps the torrent seeding.""" + arr = _arr_for_queue_removal() + + with patch( + "src.settings._instances.make_request", new=AsyncMock() + ) as mocked_request: + mocked_request.return_value = MagicMock(status_code=200) + await arr.remove_queue_item( + queue_id=7, blocklist=True, remove_from_client=False + ) + + params = mocked_request.call_args.kwargs["params"] + assert params["removeFromClient"] is False + assert params["blocklist"] is True diff --git a/tests/settings/test_jobs.py b/tests/settings/test_jobs.py new file mode 100644 index 00000000..d8d1927c --- /dev/null +++ b/tests/settings/test_jobs.py @@ -0,0 +1,45 @@ +from src.settings._jobs import JobParams + + +def test_job_params_truthiness_is_false_by_default(): + assert not JobParams() + + +def test_job_params_truthiness_reflects_enabled_flag(): + assert JobParams(enabled=True) + assert not JobParams(enabled=False) + + +class _FakeSettings: + class general: + obsolete_tag = "Obsolete" + + +def _jobs(jobs_config): + from src.settings._jobs import Jobs + + return Jobs({"jobs": jobs_config}, _FakeSettings()) + + +def test_bool_true_keeps_job_defaults(): + # Regression: JobParams.__bool__ made the pre-built defaults object falsy + # (enabled=False), so `remove_failed_imports: true` was replaced by a bare + # JobParams(enabled=True) and lost message_patterns -> AttributeError at runtime. + jobs = _jobs({"remove_failed_imports": True, "remove_stalled": True}) + assert jobs.remove_failed_imports.enabled is True + assert jobs.remove_failed_imports.message_patterns == ["*"] + assert jobs.remove_stalled.enabled is True + assert jobs.remove_stalled.max_strikes == 3 + + +def test_bool_false_keeps_job_defaults(): + jobs = _jobs({"remove_failed_imports": False}) + assert jobs.remove_failed_imports.enabled is False + assert jobs.remove_failed_imports.message_patterns == ["*"] + + +def test_dict_merges_onto_job_defaults(): + jobs = _jobs({"remove_failed_imports": {"message_patterns": ["Not an upgrade*"]}, "remove_stalled": {"max_strikes": 5}}) + assert jobs.remove_failed_imports.enabled is True + assert jobs.remove_failed_imports.message_patterns == ["Not an upgrade*"] + assert jobs.remove_stalled.max_strikes == 5 diff --git a/tests/settings/test_sabnzbd_clients.py b/tests/settings/test_sabnzbd_clients.py index af7bc337..b4ee4977 100644 --- a/tests/settings/test_sabnzbd_clients.py +++ b/tests/settings/test_sabnzbd_clients.py @@ -130,3 +130,67 @@ def test_init_missing_required_field(self, caplog): clients = SabnzbdClients(config, settings) assert len(clients) == 0 assert "Error parsing sabnzbd client config" in caplog.text + + +class TestSabnzbdSetupDegradation: + @pytest.mark.asyncio + async def test_setup_reachability_timeout_marks_transient(self): + from unittest.mock import patch + + import requests + + settings = Mock() + settings.min_versions.sabnzbd = "4.0.0" + client = SabnzbdClient( + settings=settings, base_url="http://sabnzbd:8080", api_key="key" + ) + + with patch( + "src.settings._download_clients_sabnzbd.make_request", + new_callable=AsyncMock, + side_effect=requests.exceptions.ReadTimeout("Read timed out."), + ): + result = await client.setup() + + assert result is False + assert client.ready is False + assert client.failure_kind == "transient" + + @pytest.mark.asyncio + async def test_setup_old_version_marks_definitive(self): + settings = Mock() + settings.min_versions.sabnzbd = "4.0.0" + client = SabnzbdClient( + settings=settings, base_url="http://sabnzbd:8080", api_key="key" + ) + client.check_sabnzbd_reachability = AsyncMock() + client.fetch_version = AsyncMock() + client.version = "3.0.0" + + await client.setup() + + assert client.ready is False + assert client.failure_kind == "definitive" + + +class TestSabnzbdBadKey: + @pytest.mark.asyncio + async def test_setup_bad_api_key_marks_definitive(self): + from unittest.mock import MagicMock, patch + + settings = Mock() + settings.min_versions.sabnzbd = "4.0.0" + client = SabnzbdClient( + settings=settings, base_url="http://sabnzbd:8080", api_key="bad" + ) + resp = MagicMock() + resp.json = MagicMock(return_value={"error": "API Key Incorrect"}) + with patch( + "src.settings._download_clients_sabnzbd.make_request", + new_callable=AsyncMock, + return_value=resp, + ): + await client.setup() + + assert client.ready is False + assert client.failure_kind == "definitive" diff --git a/tests/settings/test_timeout.py b/tests/settings/test_timeout.py new file mode 100644 index 00000000..fcfe965e --- /dev/null +++ b/tests/settings/test_timeout.py @@ -0,0 +1,165 @@ +"""Tests for configurable request timeouts (general + per-instance).""" +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest + +from src.settings._download_clients_qbit import QbitClient +from src.settings._download_clients_sabnzbd import SabnzbdClient +from src.settings._general import General +from src.settings._instances import ArrInstance +from src.utils.common import make_request + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def make_settings(request_timeout=15): + s = Mock() + s.general.request_timeout = request_timeout + s.general.test_run = False + s.general.ssl_verification = True + return s + + +def make_general(request_timeout=None): + cfg = {} + if request_timeout is not None: + cfg["general"] = {"request_timeout": request_timeout} + return General(cfg) + + +# --------------------------------------------------------------------------- +# General settings +# --------------------------------------------------------------------------- + +class TestGeneralRequestTimeout: + def test_default_is_15(self): + g = make_general() + assert g.request_timeout == 15 + + def test_loads_from_config(self): + g = make_general(request_timeout=60) + assert g.request_timeout == 60 + + def test_string_value_coerced_to_int(self): + g = General({"general": {"request_timeout": "30"}}) + assert g.request_timeout == 30 + assert isinstance(g.request_timeout, int) + + +# --------------------------------------------------------------------------- +# ArrInstance.timeout property +# --------------------------------------------------------------------------- + +class TestArrInstanceTimeout: + def test_falls_back_to_15_with_empty_settings(self): + arr = ArrInstance({}, "radarr", "http://radarr/", "key") + assert arr.timeout == 15 + + def test_falls_back_to_general_request_timeout(self): + arr = ArrInstance(make_settings(request_timeout=20), "radarr", "http://radarr/", "key") + assert arr.timeout == 20 + + def test_per_instance_timeout_overrides_general(self): + arr = ArrInstance(make_settings(request_timeout=20), "radarr", "http://radarr/", "key", timeout=45) + assert arr.timeout == 45 + + def test_general_timeout_change_reflected_dynamically(self): + settings = make_settings(request_timeout=20) + arr = ArrInstance(settings, "radarr", "http://radarr/", "key") + assert arr.timeout == 20 + settings.general.request_timeout = 99 + assert arr.timeout == 99 + + def test_instance_timeout_not_affected_by_general_change(self): + settings = make_settings(request_timeout=20) + arr = ArrInstance(settings, "radarr", "http://radarr/", "key", timeout=45) + settings.general.request_timeout = 99 + assert arr.timeout == 45 + + +# --------------------------------------------------------------------------- +# QbitClient.timeout property +# --------------------------------------------------------------------------- + +class TestQbitClientTimeout: + def _make_client(self, general_timeout=15, instance_timeout=None): + settings = make_settings(request_timeout=general_timeout) + kwargs = dict(settings=settings, base_url="http://qbit/", name="qBittorrent") + if instance_timeout is not None: + kwargs["timeout"] = instance_timeout + return QbitClient(**kwargs) + + def test_falls_back_to_general_request_timeout(self): + assert self._make_client(general_timeout=25).timeout == 25 + + def test_per_instance_timeout_overrides_general(self): + assert self._make_client(general_timeout=25, instance_timeout=50).timeout == 50 + + def test_general_timeout_change_reflected_dynamically(self): + client = self._make_client(general_timeout=25) + client.settings.general.request_timeout = 77 + assert client.timeout == 77 + + +# --------------------------------------------------------------------------- +# SabnzbdClient.timeout property +# --------------------------------------------------------------------------- + +class TestSabnzbdClientTimeout: + def _make_client(self, general_timeout=15, instance_timeout=None): + settings = make_settings(request_timeout=general_timeout) + kwargs = dict(settings=settings, base_url="http://sabnzbd/", api_key="key") + if instance_timeout is not None: + kwargs["timeout"] = instance_timeout + return SabnzbdClient(**kwargs) + + def test_falls_back_to_general_request_timeout(self): + assert self._make_client(general_timeout=25).timeout == 25 + + def test_per_instance_timeout_overrides_general(self): + assert self._make_client(general_timeout=25, instance_timeout=50).timeout == 50 + + def test_general_timeout_change_reflected_dynamically(self): + client = self._make_client(general_timeout=25) + client.settings.general.request_timeout = 77 + assert client.timeout == 77 + + +# --------------------------------------------------------------------------- +# make_request passes timeout through correctly +# --------------------------------------------------------------------------- + +class TestMakeRequestTimeout: + @pytest.mark.asyncio + async def test_uses_explicit_timeout(self): + settings = make_settings() + captured = {} + + async def fake_thread(fn, *args, **kwargs): + captured["timeout"] = kwargs.get("timeout") + resp = MagicMock() + resp.raise_for_status = MagicMock() + return resp + + with patch("src.utils.common.asyncio.to_thread", side_effect=fake_thread): + await make_request("get", "http://example.com", settings, timeout=42) + + assert captured["timeout"] == 42 + + @pytest.mark.asyncio + async def test_default_timeout_is_15(self): + settings = make_settings() + captured = {} + + async def fake_thread(fn, *args, **kwargs): + captured["timeout"] = kwargs.get("timeout") + resp = MagicMock() + resp.raise_for_status = MagicMock() + return resp + + with patch("src.utils.common.asyncio.to_thread", side_effect=fake_thread): + await make_request("get", "http://example.com", settings) + + assert captured["timeout"] == 15 diff --git a/tests/settings/test_timeout_config_loading.py b/tests/settings/test_timeout_config_loading.py new file mode 100644 index 00000000..1bbf4a4a --- /dev/null +++ b/tests/settings/test_timeout_config_loading.py @@ -0,0 +1,141 @@ +"""Tests that timeout values load correctly from YAML config text.""" +import textwrap +from unittest.mock import Mock + +import yaml + +from src.settings._download_clients_qbit import QbitClients +from src.settings._download_clients_sabnzbd import SabnzbdClients +from src.settings._general import General +from src.settings._instances import ArrInstances + + +def parse(text): + return yaml.safe_load(textwrap.dedent(text)) + + +def make_settings(request_timeout=15): + s = Mock() + s.general.request_timeout = request_timeout + return s + + +# --------------------------------------------------------------------------- +# General.request_timeout +# --------------------------------------------------------------------------- + +class TestGeneralFromYaml: + def test_request_timeout_loaded(self): + config = parse(""" + general: + request_timeout: 45 + """) + assert General(config).request_timeout == 45 + + def test_request_timeout_default_when_absent(self): + config = parse(""" + general: + log_level: INFO + """) + assert General(config).request_timeout == 15 + + def test_request_timeout_as_string_coerced(self): + # YAML may deliver it as int already, but verify string also works + config = {"general": {"request_timeout": "30"}} + assert General(config).request_timeout == 30 + + +# --------------------------------------------------------------------------- +# ArrInstance timeout via ArrInstances loader +# --------------------------------------------------------------------------- + +class TestArrInstanceFromYaml: + def test_instance_timeout_loaded(self): + config = parse(""" + instances: + radarr: + - base_url: "http://radarr/" + api_key: "k" + timeout: 60 + """) + instances = ArrInstances(config, make_settings()) + assert instances[0].timeout == 60 + + def test_instance_timeout_absent_uses_general(self): + config = parse(""" + instances: + radarr: + - base_url: "http://radarr/" + api_key: "k" + """) + instances = ArrInstances(config, make_settings(request_timeout=30)) + assert instances[0].timeout == 30 + + def test_mixed_instances(self): + config = parse(""" + instances: + radarr: + - base_url: "http://radarr/" + api_key: "k" + timeout: 60 + sonarr: + - base_url: "http://sonarr/" + api_key: "k" + """) + instances = ArrInstances(config, make_settings(request_timeout=20)) + radarr = next(i for i in instances if i.arr_type == "radarr") + sonarr = next(i for i in instances if i.arr_type == "sonarr") + assert radarr.timeout == 60 + assert sonarr.timeout == 20 + + +# --------------------------------------------------------------------------- +# QbitClient timeout via QbitClients loader +# --------------------------------------------------------------------------- + +class TestQbitClientFromYaml: + def test_timeout_loaded(self): + config = parse(""" + download_clients: + qbittorrent: + - base_url: "http://qbit/" + timeout: 55 + """) + clients = QbitClients(config, make_settings()) + assert clients[0].timeout == 55 + + def test_timeout_absent_uses_general(self): + config = parse(""" + download_clients: + qbittorrent: + - base_url: "http://qbit/" + """) + clients = QbitClients(config, make_settings(request_timeout=40)) + assert clients[0].timeout == 40 + + +# --------------------------------------------------------------------------- +# SabnzbdClient timeout via SabnzbdClients loader +# --------------------------------------------------------------------------- + +class TestSabnzbdClientFromYaml: + def test_timeout_loaded(self): + config = parse(""" + download_clients: + sabnzbd: + - base_url: "http://sabnzbd/" + api_key: "k" + timeout: 70 + """) + clients = SabnzbdClients(config, make_settings()) + assert clients[0].timeout == 70 + + def test_timeout_absent_uses_general(self): + config = parse(""" + download_clients: + sabnzbd: + - base_url: "http://sabnzbd/" + api_key: "k" + """) + clients = SabnzbdClients(config, make_settings(request_timeout=35)) + assert clients[0].timeout == 35 diff --git a/tests/settings/test_user_config_from_env.py b/tests/settings/test_user_config_from_env.py index 381034ce..d0b2d759 100644 --- a/tests/settings/test_user_config_from_env.py +++ b/tests/settings/test_user_config_from_env.py @@ -1,5 +1,6 @@ """Test loading the user configuration from environment variables.""" +import logging import os import textwrap from unittest.mock import patch @@ -14,6 +15,7 @@ # Single values as plain strings (not YAML block strings) LOG_LEVEL_VALUE = "VERBOSE" TIMER_VALUE = "10" +REQUEST_TIMEOUT_VALUE = "35" SSL_VERIFICATION_VALUE = "true" # List @@ -79,6 +81,7 @@ def fixture_env_vars(): env = { "LOG_LEVEL": LOG_LEVEL_VALUE, "TIMER": TIMER_VALUE, + "REQUEST_TIMEOUT": REQUEST_TIMEOUT_VALUE, "SSL_VERIFICATION": SSL_VERIFICATION_VALUE, "IGNORED_DOWNLOAD_CLIENTS": ignored_download_clients_yaml, "REMOVE_BAD_FILES": remove_bad_files_yaml, @@ -107,6 +110,7 @@ def fixture_env_vars(): [ ("general", "log_level", LOG_LEVEL_VALUE), ("general", "timer", int(TIMER_VALUE)), + ("general", "request_timeout", int(REQUEST_TIMEOUT_VALUE)), ("general", "ssl_verification", True), ( "general", @@ -134,3 +138,20 @@ def test_env_loading_parametrized( assert value == expected else: assert value == expected + + +def test_invalid_download_client_yaml_logs_actionable_error(caplog): + malformed_qbit_yaml = '- base_url: "http://qbittorrent:8080' + + with ( + patch.dict(os.environ, {"QBITTORRENT": malformed_qbit_yaml}, clear=True), + caplog.at_level(logging.ERROR, logger="src.utils.log_setup"), + ): + config = _load_from_env() + + assert config["download_clients"]["qbittorrent"] == {} + assert len(caplog.records) == 1 + message = caplog.records[0].getMessage() + assert "Could not parse QBITTORRENT as YAML" in message + assert "smart quotes or incorrect indentation" in message + assert "Configuration from this variable was ignored" in message diff --git a/tests/utils/test_common.py b/tests/utils/test_common.py new file mode 100644 index 00000000..5180629f --- /dev/null +++ b/tests/utils/test_common.py @@ -0,0 +1,58 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.utils.common import make_request + + +@pytest.mark.asyncio +async def test_make_request_uses_general_request_timeout_by_default(): + settings = MagicMock() + settings.general.test_run = False + settings.general.ssl_verification = True + settings.general.request_timeout = 42 + + mock_response = MagicMock() + mock_response.raise_for_status = MagicMock() + + with patch( + "src.utils.common.asyncio.to_thread", new_callable=AsyncMock + ) as to_thread: + to_thread.return_value = mock_response + + await make_request("get", "http://example.com/api", settings) + + assert to_thread.await_count == 1 + assert to_thread.await_args.kwargs["timeout"] == 42 + + +@pytest.mark.asyncio +async def test_make_request_allows_explicit_timeout_override(): + settings = MagicMock() + settings.general.test_run = False + settings.general.ssl_verification = True + settings.general.request_timeout = 42 + + mock_response = MagicMock() + mock_response.raise_for_status = MagicMock() + + with patch( + "src.utils.common.asyncio.to_thread", new_callable=AsyncMock + ) as to_thread: + to_thread.return_value = mock_response + + await make_request("get", "http://example.com/api", settings, timeout=7) + + assert to_thread.await_count == 1 + assert to_thread.await_args.kwargs["timeout"] == 7 + + +def test_sanitize_kwargs_redacts_authorization_header(): + """The Bearer token must never survive into logs.""" + from src.utils.common import sanitize_kwargs + + data = {"headers": {"Authorization": "Bearer qbt_supersecrettoken"}} + result = sanitize_kwargs(data) + + assert result["headers"]["Authorization"] == "[**redacted**]" + assert "qbt_supersecrettoken" not in str(result) diff --git a/tests/utils/test_queue_manager.py b/tests/utils/test_queue_manager.py index 03fc549a..c4ef0255 100644 --- a/tests/utils/test_queue_manager.py +++ b/tests/utils/test_queue_manager.py @@ -103,3 +103,28 @@ def test_format_queue_multiple_different_download_ids(mock_queue_manager): } result = mock_queue_manager.format_queue(queue_items) assert result == expected + + + +@pytest.mark.asyncio +async def test_orphans_ignore_progress_between_fetches(mock_queue_manager, monkeypatch): + # An active download changes sizeleft between the full and the normal fetch. + # It is in both, so it is not an orphan. Item 2 is only in the full queue. + full = [ + {"id": 1, "downloadId": "a", "sizeleft": 900, "detail_item_id": 10}, + {"id": 2, "downloadId": "b", "sizeleft": 500, "detail_item_id": None}, + ] + normal = [{"id": 1, "downloadId": "a", "sizeleft": 850, "detail_item_id": 10}] + + async def fake_refresh(): + return None + + async def fake_get_queue(*, full_queue=False): + return list(full) if full_queue else list(normal) + + monkeypatch.setattr(mock_queue_manager, "_refresh_queue", fake_refresh) + monkeypatch.setattr(mock_queue_manager, "_get_queue", fake_get_queue) + + result = await mock_queue_manager.get_queue_items("orphans") + + assert [item["id"] for item in result] == [2] diff --git a/tests/utils/test_startup.py b/tests/utils/test_startup.py new file mode 100644 index 00000000..8f2e1e41 --- /dev/null +++ b/tests/utils/test_startup.py @@ -0,0 +1,114 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.utils.startup import _exit_if_all_failed_definitively, retry_degraded_instances + + +def _unit(ready=False, failure_kind=None, arr_type=None, setup_result=False): + unit = MagicMock() + unit.ready = ready + unit.failure_kind = failure_kind + unit.name = "Unit" + unit.base_url = "http://unit" + unit.last_error = "some error" + unit.setup_tip = "πŸ’‘ Tip: fix it" + unit.arr_type = arr_type + unit.setup = AsyncMock(return_value=setup_result) + return unit + + +def _settings(instances=None, qbits=None, sabnzbds=None): + settings = MagicMock() + settings.instances = instances or [] + settings.download_clients.qbittorrent = qbits or [] + settings.download_clients.sabnzbd = sabnzbds or [] + settings.jobs.detect_deletions.enabled = True + return settings + + +def test_exit_when_all_units_failed_definitively(): + settings = _settings( + instances=[_unit(failure_kind="definitive")], + qbits=[_unit(failure_kind="definitive")], + ) + + with patch("src.utils.startup.wait_and_exit") as mock_exit: + _exit_if_all_failed_definitively(settings) + + mock_exit.assert_called_once() + + +def test_no_exit_when_one_unit_failed_transiently(): + settings = _settings( + instances=[_unit(failure_kind="definitive"), _unit(failure_kind="transient")], + ) + + with patch("src.utils.startup.wait_and_exit") as mock_exit: + _exit_if_all_failed_definitively(settings) + + mock_exit.assert_not_called() + + +def test_no_exit_when_one_unit_is_healthy(): + settings = _settings( + instances=[_unit(ready=True), _unit(failure_kind="definitive")], + ) + + with patch("src.utils.startup.wait_and_exit") as mock_exit: + _exit_if_all_failed_definitively(settings) + + mock_exit.assert_not_called() + + +@pytest.mark.asyncio +async def test_retry_skips_ready_and_definitive_units(caplog): + ready_unit = _unit(ready=True) + definitive_unit = _unit(failure_kind="definitive") + settings = _settings(instances=[ready_unit, definitive_unit]) + + with caplog.at_level("ERROR"): + await retry_degraded_instances(settings) + + ready_unit.setup.assert_not_awaited() + definitive_unit.setup.assert_not_awaited() + assert any("configuration error" in record.message for record in caplog.records) + + +@pytest.mark.asyncio +async def test_retry_rejoins_transient_arr_and_sets_up_watchers(): + arr = _unit(failure_kind="transient", arr_type="sonarr", setup_result=True) + settings = _settings(instances=[arr]) + watch_manager = MagicMock() + watch_manager.setup_for_arr = AsyncMock() + + await retry_degraded_instances(settings, watch_manager) + + arr.setup.assert_awaited_once() + watch_manager.setup_for_arr.assert_awaited_once_with(arr) + + +@pytest.mark.asyncio +async def test_retry_logs_warning_when_still_failing(caplog): + arr = _unit(failure_kind="transient", arr_type="sonarr", setup_result=False) + settings = _settings(instances=[arr]) + watch_manager = MagicMock() + watch_manager.setup_for_arr = AsyncMock() + + with caplog.at_level("WARNING"): + await retry_degraded_instances(settings, watch_manager) + + arr.setup.assert_awaited_once() + watch_manager.setup_for_arr.assert_not_awaited() + assert any("will retry next cycle" in record.message for record in caplog.records) + + +@pytest.mark.asyncio +async def test_retry_exits_when_everything_becomes_definitive(): + """If the only units are definitively failed, the per-cycle retry exits.""" + settings = _settings(instances=[_unit(failure_kind="definitive")]) + + with patch("src.utils.startup.wait_and_exit") as mock_exit: + await retry_degraded_instances(settings) + + mock_exit.assert_called_once()