From ad02eddfa83a84206f6615f52e9aa22c0f6799b9 Mon Sep 17 00:00:00 2001 From: juan Date: Thu, 7 May 2026 16:50:33 -0700 Subject: [PATCH 01/26] chore: Add GitHub issue templates for bugs and features MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three files under .github/ISSUE_TEMPLATE/ to structure incoming issues and reduce the back-and-forth maintainers currently spend extracting version/config/log details from terse reports. - bug_report.yml: required fields for decluttarr version, deployment method, *arr versions, download client, sanitized config, debug logs, repro steps, and expected vs. actual behavior. Auto-applies the "Bug" label. - feature_request.yml: structured around problem statement, proposed solution, alternatives considered. Auto-applies the "Feature" label. - config.yml: disables blank issues so users must pick a template, and adds two contact links β€” one to GitHub Discussions for questions/config help (which currently land in the issue tracker and clutter triage), and one to the README's FAQ section that already covers many common problems. Closes part of the chore list in discussion #345 ("Set up Issue templates"). Co-Authored-By: Claude Opus 4.7 --- .github/ISSUE_TEMPLATE/bug_report.yml | 111 +++++++++++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 10 ++ .github/ISSUE_TEMPLATE/feature_request.yml | 41 ++++++++ 3 files changed, 162 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/feature_request.yml diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 00000000..be79aa03 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,111 @@ +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](https://github.com/ManiMatter/decluttarr#dependencies--hints--faq), especially the **Dependencies & Hints & FAQ** section + - 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 version + description: | + Image tag (e.g. `latest`, `dev`) or commit hash. Find it in the startup log line `Decluttarr `. + placeholder: "e.g. latest, dev, or 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: 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: textarea + id: config + attributes: + label: Sanitized config + description: | + Your `config.yaml` (or env-var equivalents) with **API keys, passwords, and other secrets removed or redacted**. The form will render this as a code block automatically. + render: yaml + placeholder: | + general: + log_level: DEBUG + test_run: true + # ... + validations: + required: true + + - type: textarea + id: logs + attributes: + label: Debug-level logs + description: | + Set `log_level: DEBUG` in your config, reproduce the issue, and paste the relevant log section here. Please include enough context (lines before and after the error) to show what was running. + 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. From d4fc68babdda5eaf1b5afc72caf7d4b620483731 Mon Sep 17 00:00:00 2001 From: juan Date: Thu, 7 May 2026 16:58:22 -0700 Subject: [PATCH 02/26] chore: Add OS, test_run, frequency, and log-sanitization fields to bug template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review pass on the templates flagged four gaps that would otherwise be extracted via comments. Adding upfront: - Host OS / architecture dropdown (Linux x86_64, Linux ARM, macOS, Windows, Other) β€” path-related bugs and qBit issues sometimes behave differently on Windows or ARM (cf. #308 Windows path mappings). - test_run state dropdown β€” removal-related bugs are very different in test_run=true vs false; capturing this avoids the "wait, were removals actually happening?" round-trip. - Reproduction frequency dropdown (Always / Sometimes / Once / Unknown) β€” affects triage priority and whether the report is actionable. - Log-sanitization warning: decluttarr already redacts X-Api-Key headers but not ?apikey=... query strings or session cookies, so the description now explicitly tells reporters to scrub those. All four are required fields. Field ordering follows a setup β†’ context β†’ detail narrative. Co-Authored-By: Claude Opus 4.7 --- .github/ISSUE_TEMPLATE/bug_report.yml | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index be79aa03..64899276 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -36,6 +36,21 @@ body: 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: @@ -54,6 +69,32 @@ body: 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: config attributes: @@ -75,6 +116,8 @@ body: label: Debug-level logs description: | Set `log_level: DEBUG` in your config, reproduce the issue, and paste the relevant log section here. Please include enough context (lines before and after the error) to show what was running. + + **Sanitize before pasting:** scrub any `?apikey=...` query parameters, `Authorization:` headers, session cookies, and arr/qBit URLs you'd rather not share. Decluttarr already redacts most `X-Api-Key` headers but not query-string keys. render: shell validations: required: true From f1cc08c1176a4c8bd183403218b667642167ecdf Mon Sep 17 00:00:00 2001 From: juan Date: Sun, 10 May 2026 09:29:33 -0700 Subject: [PATCH 03/26] =?UTF-8?q?chore:=20Fix=20version=20field=20descript?= =?UTF-8?q?ion=20=E2=80=94=20no=20startup=20log=20line=20exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @ManiMatter's review on #357: the previous description told reporters to find their version in a log line `Decluttarr ` that doesn't actually exist. image_tag and short_commit_id come from env vars set by the Docker build pipeline (only surfaced in the web UI footer once #337 lands), not from any startup log message. Replace with practical instructions for both deployment paths: - Docker users: the image tag they pulled (latest/dev) - Local Python users: short commit hash from git rev-parse --short HEAD Co-Authored-By: Claude Opus 4.7 --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 64899276..abe5f9b5 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -18,7 +18,7 @@ body: attributes: label: Decluttarr version description: | - Image tag (e.g. `latest`, `dev`) or commit hash. Find it in the startup log line `Decluttarr `. + The Docker image tag you pulled (typically `latest` or `dev`), or for local Python users the short commit hash from `git rev-parse --short HEAD`. placeholder: "e.g. latest, dev, or 51b6a4c" validations: required: true From f1ab69d631ba696d70289e637dcc89cd19346d75 Mon Sep 17 00:00:00 2001 From: juan Date: Sun, 10 May 2026 11:12:48 -0700 Subject: [PATCH 04/26] chore: Restructure bug template per @ManiMatter feedback on #357 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ManiMatter pointed out that decluttarr already prints a complete configuration dump at startup under the `πŸ› οΈ Decluttarr - Settings πŸ› οΈ` header (when log level is DEBUG or VERBOSE) β€” covering image_tag, short_commit_id, all general settings, jobs, instances, and download clients with API keys already masked as `*****`. Asking for a separate "Sanitized config" field is therefore redundant with what reporters will paste in the logs field anyway. Changes: - Drop the "Sanitized config" textarea entirely - Rewrite "Debug-level logs" β†’ "Decluttarr startup banner + debug logs" with the three-step workflow @ManiMatter described: 1. disable jobs not related to the issue 2. set log_level: DEBUG 3. paste everything from the startup banner through the failing cycle - Tweak the version-field description to mention that the banner also contains image_tag / short_commit_id, so reporters can cross-check Net change: 13 fields instead of 14, less duplication, more useful content per report. Co-Authored-By: Claude Opus 4.7 --- .github/ISSUE_TEMPLATE/bug_report.yml | 29 +++++++++------------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index abe5f9b5..c48c8a17 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -18,7 +18,7 @@ body: attributes: label: Decluttarr version description: | - The Docker image tag you pulled (typically `latest` or `dev`), or for local Python users the short commit hash from `git rev-parse --short HEAD`. + The Docker image tag you pulled (typically `latest` or `dev`) or β€” for local Python users β€” the short commit hash from `git rev-parse --short HEAD`. Also visible in the `πŸ› οΈ Decluttarr - Settings πŸ› οΈ` startup banner under `image_tag` / `short_commit_id` (which you'll be pasting in the logs field below). placeholder: "e.g. latest, dev, or 51b6a4c" validations: required: true @@ -95,29 +95,20 @@ body: validations: required: true - - type: textarea - id: config - attributes: - label: Sanitized config - description: | - Your `config.yaml` (or env-var equivalents) with **API keys, passwords, and other secrets removed or redacted**. The form will render this as a code block automatically. - render: yaml - placeholder: | - general: - log_level: DEBUG - test_run: true - # ... - validations: - required: true - - type: textarea id: logs attributes: - label: Debug-level logs + label: Decluttarr startup banner + debug logs description: | - Set `log_level: DEBUG` in your config, reproduce the issue, and paste the relevant log section here. Please include enough context (lines before and after the error) to show what was running. + 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 `*****`. - **Sanitize before pasting:** scrub any `?apikey=...` query parameters, `Authorization:` headers, session cookies, and arr/qBit URLs you'd rather not share. Decluttarr already redacts most `X-Api-Key` headers but not query-string keys. + **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 From 0e885571f00d77b82cd33e9e9d509f83d965235e Mon Sep 17 00:00:00 2001 From: juan Date: Mon, 11 May 2026 15:07:21 -0700 Subject: [PATCH 05/26] chore: Require commit hash for version field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ManiMatter pointed out that accepting `latest` / `dev` tags as version values is misleading since those tags move quickly β€” for reproducible bugs we need the specific commit. Drop the tag aliases, ask for the short commit hash directly, point Docker users at the startup banner's `short_commit_id` and local Python users at `git rev-parse --short HEAD`. Co-Authored-By: Claude Opus 4.7 --- .github/ISSUE_TEMPLATE/bug_report.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index c48c8a17..2c005d1c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -16,10 +16,13 @@ body: - type: input id: version attributes: - label: Decluttarr version + label: Decluttarr commit hash description: | - The Docker image tag you pulled (typically `latest` or `dev`) or β€” for local Python users β€” the short commit hash from `git rev-parse --short HEAD`. Also visible in the `πŸ› οΈ Decluttarr - Settings πŸ› οΈ` startup banner under `image_tag` / `short_commit_id` (which you'll be pasting in the logs field below). - placeholder: "e.g. latest, dev, or 51b6a4c" + 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 From 2ede62fbb61871c66add7d53f14cf58519b8dbd7 Mon Sep 17 00:00:00 2001 From: Alexis Morain Date: Wed, 17 Jun 2026 12:35:18 +0200 Subject: [PATCH 06/26] feat(remove_metadata_missing): detect stuck metadata for non-qBittorrent clients The remove_metadata_missing job only matches the "qBittorrent is downloading metadata" message that qBittorrent surfaces in the *arr queue. Clients such as Transmission or Deluge surface no such message, so a torrent stuck fetching metadata (status "queued", size 0) is never detected and lingers indefinitely (see #57). Add an opt-in, client-agnostic fallback `detect_via_missing_size` (default False, so existing qBittorrent behaviour is unchanged): when enabled, queued items whose size is not yet known (size 0) are also flagged. Reuses the existing max_strikes debounce and blocklist/redownload flow. Adds a QueueManager.filter_missing_size helper, tests for on/off/dedup/partial cases, and README docs. Co-Authored-By: Claude Opus 4.8 --- README.md | 4 +- src/jobs/remove_metadata_missing.py | 17 +++- src/settings/_jobs.py | 4 + src/utils/queue_manager.py | 18 ++++ tests/jobs/test_remove_metadata_missing.py | 99 ++++++++++++++++++++++ 5 files changed, 140 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9d2d9bc7..50f44286 100644 --- a/README.md +++ b/README.md @@ -551,11 +551,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 diff --git a/src/jobs/remove_metadata_missing.py b/src/jobs/remove_metadata_missing.py index 31e86417..07c1150e 100644 --- a/src/jobs/remove_metadata_missing.py +++ b/src/jobs/remove_metadata_missing.py @@ -6,5 +6,20 @@ class RemoveMetadataMissing(RemovalJob): 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". + # Debounced by max_strikes like the rest of this job. + if getattr(self.job, "detect_via_missing_size", False): + seen = {id(item) for item in affected_items} + for item in self.queue_manager.filter_missing_size(self.queue): + if id(item) not in seen: + affected_items.append(item) + seen.add(id(item)) + + return affected_items diff --git a/src/settings/_jobs.py b/src/settings/_jobs.py index c71eaabc..f5f0e7dd 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() @@ -86,6 +89,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() diff --git a/src/utils/queue_manager.py b/src/utils/queue_manager.py index e05f1335..f7375255 100644 --- a/src/utils/queue_manager.py +++ b/src/utils/queue_manager.py @@ -241,3 +241,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/tests/jobs/test_remove_metadata_missing.py b/tests/jobs/test_remove_metadata_missing.py index 7f92c37b..1ff2863f 100644 --- a/tests/jobs/test_remove_metadata_missing.py +++ b/tests/jobs/test_remove_metadata_missing.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock + import pytest from src.jobs.remove_metadata_missing import RemoveMetadataMissing @@ -91,3 +93,100 @@ async def test_find_affected_items(queue_data, expected_download_ids): # 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) + + # Act and Assert + await shared_test_affected_items(removal_job, expected_download_ids) From 7d700cd232d707767a0a3c28f800097fa1a4a025 Mon Sep 17 00:00:00 2001 From: Tomer Horowitz Date: Sat, 28 Feb 2026 21:40:33 +0200 Subject: [PATCH 07/26] Fix detect_deletions gating and harden timeout handling --- README.md | 9 +++ config/config_example.yaml | 1 + main.py | 18 ++++- src/job_manager.py | 56 ++++++++++++++-- src/settings/_general.py | 4 ++ src/settings/_jobs.py | 4 ++ src/settings/_user_config.py | 1 + src/utils/common.py | 7 +- tests/jobs/test_job_manager.py | 73 +++++++++++++++++++++ tests/settings/test_general.py | 13 ++++ tests/settings/test_jobs.py | 10 +++ tests/settings/test_user_config_from_env.py | 3 + tests/utils/test_common.py | 43 ++++++++++++ 13 files changed, 233 insertions(+), 9 deletions(-) create mode 100644 tests/jobs/test_job_manager.py create mode 100644 tests/settings/test_general.py create mode 100644 tests/settings/test_jobs.py create mode 100644 tests/utils/test_common.py diff --git a/README.md b/README.md index 9d2d9bc7..ec1d8171 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ Looking to **upgrade from V1 to V2**? Look [here](#upgrading-from-v1-to-v2) - [LOG_LEVEL](#log_level) - [TEST_RUN](#test_run) - [TIMER](#timer) + - [REQUEST_TIMEOUT](#request_timeout) - [SSL_VERIFICATION](#ssl_verification) - [IGNORE_DOWNLOAD_CLIENTS](#ignore_download_clients) - [PRIVATE_TRACKER_HANDLING / PUBLIC_TRACKER_HANDLING](#private_tracker_handling--public_tracker_handling) @@ -193,6 +194,7 @@ services: LOG_LEVEL: INFO TEST_RUN: True TIMER: 10 + # REQUEST_TIMEOUT: 15 # IGNORED_DOWNLOAD_CLIENTS: > # - emulerr # SSL_VERIFICATION: true @@ -399,6 +401,13 @@ Configures the general behavior of the application (across all features) - Unit: Minutes - Is Mandatory: No (Defaults to 10) +#### REQUEST_TIMEOUT + +- Timeout used for HTTP/API requests to *arr and download clients +- Type: Integer or Float +- Unit: Seconds +- Is Mandatory: No (Defaults to 15) + #### SSL_VERIFICATION - Turns SSL certificate verification on or off for all API calls diff --git a/config/config_example.yaml b/config/config_example.yaml index 71ec755d..37f142bb 100644 --- a/config/config_example.yaml +++ b/config/config_example.yaml @@ -2,6 +2,7 @@ general: log_level: INFO test_run: true timer: 10 + # request_timeout: 15 # Optional: timeout for all HTTP/API calls in seconds # ignored_download_clients: ["emulerr"] # ssl_verification: false # Optional: Defaults to true # private_tracker_handling: "obsolete_tag" # remove, skip, obsolete_tag. Optional. Default: remove diff --git a/main.py b/main.py index 6b989ad5..b51140a3 100644 --- a/main.py +++ b/main.py @@ -52,7 +52,7 @@ async def wait_next_run(): async def main(): await launch_steps(settings) - if settings.jobs.detect_deletions: + if settings.jobs.detect_deletions.enabled: await WatcherManager(settings).setup() # Start Cleaning while True: @@ -60,7 +60,13 @@ async def main(): # Refresh qBit Cookies (SABnzbd doesn't need cookie refresh) for qbit in settings.download_clients.qbittorrent: - await qbit.refresh_cookie() + try: + await qbit.refresh_cookie() + except Exception as err: # noqa: BLE001 + logger.error( + f"Error while refreshing cookie for {qbit.name} ({qbit.base_url}): {err}", + exc_info=True, + ) # Run script for each instance for arr in settings.instances: @@ -68,7 +74,13 @@ async def main(): 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/job_manager.py b/src/job_manager.py index 978d10bb..7c868f82 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 @@ -56,7 +71,9 @@ async def run_download_client_jobs(self): for download_client_job in download_client_jobs: if download_client_job.job.enabled: - items_detected += await download_client_job.run() + items_detected += await self._run_download_client_job( + download_client_job, client + ) return items_detected @@ -143,6 +160,37 @@ async def _check_client_connection_status(self, clients): return False return True + async def _run_arr_job_group(self, label, job_runner): + """Run one ARR job group and keep the main loop alive on failures.""" + try: + await job_runner() + except requests.exceptions.RequestException as err: + logger.error( + f"{label}: request error on {self.arr.name} ({self.arr.base_url}): {err}", + exc_info=True, + ) + except Exception as err: # noqa: BLE001 + logger.error( + f"{label}: unexpected error on {self.arr.name} ({self.arr.base_url}): {err}", + exc_info=True, + ) + + async def _run_download_client_job(self, download_client_job, client): + """Run one download-client job and keep the loop alive on failures.""" + try: + return await download_client_job.run() + except requests.exceptions.RequestException as err: + logger.error( + f"Download-client job '{download_client_job.job_name}' failed on {client.name} ({client.base_url}) due to request error: {err}", + exc_info=True, + ) + except Exception as err: # noqa: BLE001 + logger.error( + f"Download-client job '{download_client_job.job_name}' failed on {client.name} ({client.base_url}): {err}", + exc_info=True, + ) + return 0 + def _get_removal_jobs(self): """ Return a list of enabled removal job instances based on the provided settings. diff --git a/src/settings/_general.py b/src/settings/_general.py index ccd42a6a..9d8917bf 100644 --- a/src/settings/_general.py +++ b/src/settings/_general.py @@ -11,6 +11,7 @@ class General: log_level: str = "INFO" test_run: bool = False timer: float = 10.0 + request_timeout: float = 15.0 ssl_verification: bool = True ignored_download_clients: list = [] private_tracker_handling: str = "remove" @@ -23,6 +24,9 @@ def __init__(self, config): self.log_level = general_config.get("log_level", self.log_level.upper()) self.test_run = general_config.get("test_run", self.test_run) self.timer = general_config.get("timer", self.timer) + self.request_timeout = general_config.get( + "request_timeout", self.request_timeout + ) self.ssl_verification = general_config.get( "ssl_verification", self.ssl_verification ) diff --git a/src/settings/_jobs.py b/src/settings/_jobs.py index c71eaabc..410d3d2b 100644 --- a/src/settings/_jobs.py +++ b/src/settings/_jobs.py @@ -44,6 +44,10 @@ def _remove_none_attributes(self): if getattr(self, attr) is None: delattr(self, attr) + def __bool__(self): + """Allow direct truthiness checks to reflect whether this job is enabled.""" + return bool(getattr(self, "enabled", False)) + class JobDefaults: """Represents default job settings.""" diff --git a/src/settings/_user_config.py b/src/settings/_user_config.py index 6cc4ac83..7ba74f3b 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", diff --git a/src/utils/common.py b/src/utils/common.py index 62be171f..dee9731b 100644 --- a/src/utils/common.py +++ b/src/utils/common.py @@ -42,7 +42,7 @@ async def make_request( method: str, endpoint: str, settings, - timeout: int = 15, + timeout: float | None = None, *, log_error=True, **kwargs, @@ -51,6 +51,9 @@ 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 +77,7 @@ async def make_request( endpoint, **kwargs, verify=settings.general.ssl_verification, - timeout=timeout, + timeout=request_timeout, ) response.raise_for_status() return response diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py new file mode 100644 index 00000000..c555976d --- /dev/null +++ b/tests/jobs/test_job_manager.py @@ -0,0 +1,73 @@ +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +import requests + +from src.job_manager import JobManager + + +@pytest.mark.asyncio +async def test_run_jobs_keeps_running_when_one_group_raises_request_error(): + settings = MagicMock() + manager = JobManager(settings) + arr = MagicMock() + arr.name = "Sonarr" + arr.base_url = "http://sonarr:8989" + + with ( + patch.object( + manager, + "removal_jobs", + AsyncMock(side_effect=requests.exceptions.ReadTimeout("timed out")), + ), + patch.object(manager, "search_jobs", AsyncMock()) as search_jobs, + ): + await manager.run_jobs(arr) + + search_jobs.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_run_download_client_jobs_handles_per_job_request_errors(): + settings = MagicMock() + client = MagicMock() + client.name = "qBittorrent" + client.base_url = "http://qbittorrent:8080" + settings.download_clients.qbittorrent = [client] + settings.download_clients.sabnzbd = [] + + manager = JobManager(settings) + failing_job = MagicMock() + failing_job.job.enabled = True + failing_job.job_name = "remove_done_seeding" + failing_job.run = AsyncMock(side_effect=requests.exceptions.ReadTimeout("timed out")) + + with ( + patch.object(manager, "_download_clients_connected", AsyncMock(return_value=True)), + patch.object( + manager, + "_get_download_client_jobs_for_client", + MagicMock(return_value=[failing_job]), + ), + ): + result = await manager.run_download_client_jobs() + + assert result == 0 + failing_job.run.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_run_download_client_jobs_handles_connection_check_request_errors(): + settings = MagicMock() + settings.download_clients.qbittorrent = [] + settings.download_clients.sabnzbd = [] + manager = JobManager(settings) + + with patch.object( + manager, + "_download_clients_connected", + AsyncMock(side_effect=requests.exceptions.ReadTimeout("timed out")), + ): + result = await manager.run_download_client_jobs() + + assert result is None diff --git a/tests/settings/test_general.py b/tests/settings/test_general.py new file mode 100644 index 00000000..52aa0753 --- /dev/null +++ b/tests/settings/test_general.py @@ -0,0 +1,13 @@ +from src.settings._general import General + + +def test_request_timeout_defaults_to_15_seconds(): + general = General({}) + + assert general.request_timeout == 15.0 + + +def test_request_timeout_accepts_numeric_strings(): + general = General({"general": {"request_timeout": "42"}}) + + assert general.request_timeout == 42.0 diff --git a/tests/settings/test_jobs.py b/tests/settings/test_jobs.py new file mode 100644 index 00000000..9d4c6144 --- /dev/null +++ b/tests/settings/test_jobs.py @@ -0,0 +1,10 @@ +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) diff --git a/tests/settings/test_user_config_from_env.py b/tests/settings/test_user_config_from_env.py index 381034ce..b35f345f 100644 --- a/tests/settings/test_user_config_from_env.py +++ b/tests/settings/test_user_config_from_env.py @@ -14,6 +14,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 +80,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 +109,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", diff --git a/tests/utils/test_common.py b/tests/utils/test_common.py new file mode 100644 index 00000000..9eeef649 --- /dev/null +++ b/tests/utils/test_common.py @@ -0,0 +1,43 @@ +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 From 2e3948556cd7ea0d2c7bcfa04124363d2973f6b2 Mon Sep 17 00:00:00 2001 From: JR Hager Date: Wed, 8 Jul 2026 18:46:31 -0600 Subject: [PATCH 08/26] Apply black formatting to files touched by this PR Co-Authored-By: Claude Fable 5 --- src/utils/common.py | 4 +++- tests/jobs/test_job_manager.py | 8 ++++++-- tests/utils/test_common.py | 8 ++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/utils/common.py b/src/utils/common.py index dee9731b..3348da00 100644 --- a/src/utils/common.py +++ b/src/utils/common.py @@ -53,7 +53,9 @@ async def make_request( 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) + 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"): diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py index c555976d..7a1d774f 100644 --- a/tests/jobs/test_job_manager.py +++ b/tests/jobs/test_job_manager.py @@ -40,10 +40,14 @@ async def test_run_download_client_jobs_handles_per_job_request_errors(): 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")) + 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, "_download_clients_connected", AsyncMock(return_value=True) + ), patch.object( manager, "_get_download_client_jobs_for_client", diff --git a/tests/utils/test_common.py b/tests/utils/test_common.py index 9eeef649..9b8afc0a 100644 --- a/tests/utils/test_common.py +++ b/tests/utils/test_common.py @@ -15,7 +15,9 @@ async def test_make_request_uses_general_request_timeout_by_default(): mock_response = MagicMock() mock_response.raise_for_status = MagicMock() - with patch("src.utils.common.asyncio.to_thread", new_callable=AsyncMock) as to_thread: + 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) @@ -34,7 +36,9 @@ async def test_make_request_allows_explicit_timeout_override(): mock_response = MagicMock() mock_response.raise_for_status = MagicMock() - with patch("src.utils.common.asyncio.to_thread", new_callable=AsyncMock) as to_thread: + 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) From 2df9b38ade9dac10e762efceeb8b6b0cd6fa54fc Mon Sep 17 00:00:00 2001 From: Ada Lyons Date: Sun, 19 Apr 2026 16:04:15 -0700 Subject: [PATCH 09/26] Adding timeout property to instances. --- config/config_example.yaml | 3 + src/settings/_download_clients_qbit.py | 37 +++- src/settings/_download_clients_sabnzbd.py | 20 ++- src/settings/_general.py | 2 + src/settings/_instances.py | 25 ++- src/utils/queue_manager.py | 2 + src/utils/wanted_manager.py | 2 + tests/settings/test_instances.py | 1 + tests/settings/test_timeout.py | 165 ++++++++++++++++++ tests/settings/test_timeout_config_loading.py | 141 +++++++++++++++ 10 files changed, 383 insertions(+), 15 deletions(-) create mode 100644 tests/settings/test_timeout.py create mode 100644 tests/settings/test_timeout_config_loading.py diff --git a/config/config_example.yaml b/config/config_example.yaml index 37f142bb..8afc5ebd 100644 --- a/config/config_example.yaml +++ b/config/config_example.yaml @@ -5,6 +5,7 @@ general: # request_timeout: 15 # Optional: timeout for all HTTP/API calls in seconds # ignored_download_clients: ["emulerr"] # ssl_verification: false # Optional: Defaults to true + # request_timeout: 15 # Optional: Request timeout in seconds. Defaults to 15. Can be overridden per instance. # private_tracker_handling: "obsolete_tag" # remove, skip, obsolete_tag. Optional. Default: remove # public_tracker_handling: "remove" # remove, skip, obsolete_tag. Optional. Default: remove # obsolete_tag: "Obsolete" # optional. Default: "Obsolete" @@ -53,6 +54,7 @@ instances: sonarr: - base_url: "http://sonarr:8989" api_key: "xxxx" + # timeout: 30 # Optional: Overrides general timeout for this instance. radarr: - base_url: "http://radarr:7878" api_key: "xxxx" @@ -72,6 +74,7 @@ download_clients: # username: xxxx # (optional -> if not provided, assuming not needed) # password: xxxx # (optional -> if not provided, assuming not needed) # 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/src/settings/_download_clients_qbit.py b/src/settings/_download_clients_qbit.py index 431a7b5f..6d2c71a5 100644 --- a/src/settings/_download_clients_qbit.py +++ b/src/settings/_download_clients_qbit.py @@ -47,8 +47,10 @@ def __init__( username: str = None, password: str = None, name: str = None, + timeout: int | None = 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'." @@ -68,6 +70,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)): @@ -95,6 +104,7 @@ def _connection_error(): "post", endpoint, self.settings, + timeout=self.timeout, data=data, headers=headers, ignore_test_run=True, @@ -133,6 +143,7 @@ async def fetch_version(self): "get", endpoint, self.settings, + timeout=self.timeout, cookies=self.cookie, ) self.version = response.text[1:] # Remove the '_v' prefix @@ -161,7 +172,7 @@ async def create_tag(self, tag: str): "_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, cookies=self.cookie) current_tags = response.json() if tag not in current_tags: @@ -171,6 +182,7 @@ async def create_tag(self, tag: str): "post", self.api_url + "/torrents/createTags", self.settings, + timeout=self.timeout, data=data, cookies=self.cookie, ) @@ -196,6 +208,7 @@ async def set_unwanted_folder(self): "get", endpoint, self.settings, + timeout=self.timeout, cookies=self.cookie, ) qbit_settings = response.json() @@ -209,6 +222,7 @@ async def set_unwanted_folder(self): "post", self.api_url + "/app/setPreferences", self.settings, + timeout=self.timeout, data=data, cookies=self.cookie, ) @@ -229,6 +243,7 @@ async def check_qbit_reachability(self): "post", endpoint, self.settings, + timeout=self.timeout, data=data, headers=headers, log_error=False, @@ -251,6 +266,7 @@ async def check_connected(self): "get", self.api_url + "/sync/maindata", self.settings, + timeout=self.timeout, cookies=self.cookie, ) ).json() @@ -353,6 +369,7 @@ async def set_tag(self, tags, hashes): "post", self.api_url + "/torrents/addTags", self.settings, + timeout=self.timeout, data=data, cookies=self.cookie, ) @@ -376,6 +393,7 @@ 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, ) @@ -395,12 +413,13 @@ 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, + cookies=self.cookie, + ) return response.json() @@ -411,6 +430,7 @@ 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, ) @@ -429,6 +449,7 @@ 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, ) @@ -440,6 +461,7 @@ async def set_bandwidth_usage(self): method="get", endpoint=self.api_url + "/transfer/info", settings=self.settings, + timeout=self.timeout, cookies=self.cookie, ) records = extract_json_from_response(response) @@ -476,6 +498,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, ) diff --git a/src/settings/_download_clients_sabnzbd.py b/src/settings/_download_clients_sabnzbd.py index 49184e58..2c272995 100644 --- a/src/settings/_download_clients_sabnzbd.py +++ b/src/settings/_download_clients_sabnzbd.py @@ -43,8 +43,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 +70,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 +89,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( @@ -109,6 +118,7 @@ async def check_sabnzbd_reachability(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, log_error=False, ignore_test_run=True, @@ -129,6 +139,7 @@ async def check_connected(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) status_data = response.json() @@ -160,6 +171,7 @@ async def get_queue_items(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) queue_data = response.json() @@ -175,6 +187,7 @@ async def get_history_items(self): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) history_data = response.json() @@ -196,6 +209,7 @@ async def remove_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -215,6 +229,7 @@ async def pause_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -234,6 +249,7 @@ async def resume_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -252,6 +268,7 @@ async def retry_download(self, nzo_id: str): "get", self.api_url, self.settings, + timeout=self.timeout, params=params, ) @@ -308,6 +325,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 9d8917bf..dad405e4 100644 --- a/src/settings/_general.py +++ b/src/settings/_general.py @@ -13,6 +13,7 @@ class General: 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" @@ -30,6 +31,7 @@ def __init__(self, config): 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..67640d72 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -122,6 +122,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: @@ -135,7 +136,7 @@ class ArrInstance: version: str = None name: str = None - 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 +148,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("/") @@ -163,11 +165,17 @@ def __init__(self, settings, arr_type: str, base_url: str, api_key: str): 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) @@ -216,6 +224,7 @@ async def _check_reachability(self): "get", endpoint, self.settings, + timeout=self.timeout, headers=headers, log_error=False, ) @@ -264,7 +273,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): @@ -332,6 +341,7 @@ async def remove_queue_item(self, queue_id, *, blocklist=False): "delete", endpoint, self.settings, + timeout=self.timeout, headers=headers, params=query, ) @@ -345,27 +355,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 +393,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/utils/queue_manager.py b/src/utils/queue_manager.py index e05f1335..ba78fa17 100644 --- a/src/utils/queue_manager.py +++ b/src/utils/queue_manager.py @@ -75,6 +75,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 +110,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}, ) 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/settings/test_instances.py b/tests/settings/test_instances.py index 17934992..2f150635 100644 --- a/tests/settings/test_instances.py +++ b/tests/settings/test_instances.py @@ -39,6 +39,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 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 From 3732748410318145f281b881abdc065911a0bac6 Mon Sep 17 00:00:00 2001 From: Ada Lyons Date: Sun, 19 Apr 2026 16:18:04 -0700 Subject: [PATCH 10/26] Adding build permission --- .github/workflows/build.yml | 3 +++ 1 file changed, 3 insertions(+) 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: From 852cea5f6d12f5647a905e24bdcbeed08bfa77df Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Fri, 10 Jul 2026 13:59:06 -0400 Subject: [PATCH 11/26] fix: handle qBit check_connected 500 errors gracefully Catch HTTPError in qBit check_connected to prevent app crash on 500 responses (e.g. rdt-client). Treats the error as a disconnected client, skips the cycle, and retries next run. Closes #348 --- src/settings/_download_clients_qbit.py | 29 ++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/settings/_download_clients_qbit.py b/src/settings/_download_clients_qbit.py index 6d2c71a5..5b7cc491 100644 --- a/src/settings/_download_clients_qbit.py +++ b/src/settings/_download_clients_qbit.py @@ -260,17 +260,24 @@ async def check_connected(self): 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, - timeout=self.timeout, - 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, + cookies=self.cookie, + ) + ).json() + )["server_state"]["connection_status"] + except Exception: + logger.warning( + ">>> %s: Failed to reach /sync/maindata. Treating as disconnected.", + self.name, + ) + return False if qbit_connection_status == "disconnected": return False return True From cf225b8ad3ecc7cf82b32fea7188571b45bc2545 Mon Sep 17 00:00:00 2001 From: JR Hager Date: Wed, 8 Jul 2026 19:14:00 -0600 Subject: [PATCH 12/26] Degrade failing instances at startup instead of exiting the whole app Previously any single instance failing its startup check terminated decluttarr via wait_and_exit(), taking healthy instances down with it and causing container crash-loops on slow servers (#317 follow-up). Now each unit (arr, qBittorrent, SABnzbd) records a readiness state: - Transient failures (timeout, connection, 5xx, unknown) degrade the instance; setup is re-attempted every timer cycle and the instance rejoins automatically (including its detect_deletions watchers). - Definitive config errors (401/403, wrong username/password, bad SABnzbd api key, non-English UI, client version too old) degrade the instance with a per-cycle ERROR + tip; they are not retried since they cannot heal without user action (and retrying a bad password would get the IP banned by qBittorrent). - The app exits when nothing is configured, or when every configured unit has failed definitively - re-checked each cycle, not only at launch. Degraded download clients are skipped everywhere a job would call them (bandwidth checks, obsolete-tagging, bad-file handling) via a ready_only lookup. Removal jobs additionally fail closed: a download whose configured client is degraded is left untouched rather than deleted, since its protection status (protected tag, private/public tracker) cannot be verified while the client is down. Wrong-arr-type and arr-version-too-old keep their existing log-and-continue behavior. Repeated identical setup failures log a single-line skip instead of the full error block each cycle. Also fixes main.py handing the deletion watchers to a throwaway WatcherManager, which left terminate() stopping an instance that owned no observers. --- main.py | 11 +- src/deletion_handler/deletion_handler.py | 59 +++++--- src/job_manager.py | 5 + src/jobs/removal_handler.py | 6 +- src/jobs/removal_job.py | 31 ++++ src/jobs/remove_bad_files.py | 2 +- src/jobs/remove_slow.py | 2 +- src/settings/_download_clients.py | 9 +- src/settings/_download_clients_qbit.py | 78 +++++++--- src/settings/_download_clients_sabnzbd.py | 63 ++++++-- src/settings/_instances.py | 42 +++++- src/utils/common.py | 12 ++ src/utils/startup.py | 69 +++++++++ .../deletion_handler/test_deletion_handler.py | 18 +++ tests/jobs/test_removal_handler.py | 20 +++ tests/jobs/test_removal_job.py | 55 +++++++ tests/jobs/test_remove_slow.py | 42 ++++++ tests/settings/test_download_clients.py | 46 ++++++ tests/settings/test_download_clients_qbit.py | 75 +++++++++- tests/settings/test_instances.py | 138 ++++++++++++++++++ tests/settings/test_sabnzbd_clients.py | 64 ++++++++ tests/utils/test_startup.py | 114 +++++++++++++++ 22 files changed, 886 insertions(+), 75 deletions(-) create mode 100644 tests/jobs/test_removal_job.py create mode 100644 tests/settings/test_download_clients.py create mode 100644 tests/utils/test_startup.py diff --git a/main.py b/main.py index b51140a3..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) @@ -53,13 +53,18 @@ async def main(): await launch_steps(settings) if settings.jobs.detect_deletions.enabled: - await WatcherManager(settings).setup() + 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: + if not qbit.ready: + continue try: await qbit.refresh_cookie() except Exception as err: # noqa: BLE001 @@ -70,6 +75,8 @@ async def main(): # 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("") diff --git a/src/deletion_handler/deletion_handler.py b/src/deletion_handler/deletion_handler.py index 2086fd8d..aa1511f3 100644 --- a/src/deletion_handler/deletion_handler.py +++ b/src/deletion_handler/deletion_handler.py @@ -99,35 +99,52 @@ 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", + "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 7c868f82..223f4aa2 100644 --- a/src/job_manager.py +++ b/src/job_manager.py @@ -56,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, @@ -150,6 +153,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", ) diff --git a/src/jobs/removal_handler.py b/src/jobs/removal_handler.py index 5af6f684..122c1930 100644 --- a/src/jobs/removal_handler.py +++ b/src/jobs/removal_handler.py @@ -44,6 +44,8 @@ async def _tag_as_obsolete(self, affected_download, download_id): 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 +57,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_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/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 5b7cc491..dc1d8b87 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, @@ -160,7 +171,7 @@ 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.", @@ -228,7 +239,7 @@ async def set_unwanted_folder(self): ) 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", @@ -239,7 +250,7 @@ async def check_qbit_reachability(self): "password": getattr(self, "password", ""), } headers = {"content-type": "application/x-www-form-urlencoded"} - await make_request( + response = await make_request( "post", endpoint, self.settings, @@ -249,11 +260,21 @@ async def check_qbit_reachability(self): 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 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.""" @@ -283,26 +304,38 @@ async def check_connected(self): 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 - # 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.""" @@ -429,7 +462,6 @@ async def get_torrent_properties(self, qbit_hash): ) 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") diff --git a/src/settings/_download_clients_sabnzbd.py b/src/settings/_download_clients_sabnzbd.py index 2c272995..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, @@ -105,16 +112,16 @@ 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, @@ -123,11 +130,24 @@ async def check_sabnzbd_reachability(self): 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.""" @@ -148,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.""" diff --git a/src/settings/_instances.py b/src/settings/_instances.py index 67640d72..99bd772b 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 @@ -42,6 +47,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 +58,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): @@ -135,6 +145,10 @@ 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, timeout: int | None = None): if not base_url: @@ -186,7 +200,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.""" @@ -243,11 +258,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) @@ -260,10 +276,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.""" diff --git a/src/utils/common.py b/src/utils/common.py index 3348da00..4bcab1df 100644 --- a/src/utils/common.py +++ b/src/utils/common.py @@ -100,6 +100,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/startup.py b/src/utils/startup.py index be4ed918..535ab331 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", "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/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_removal_handler.py b/tests/jobs/test_removal_handler.py index 08a754c1..0827ede5 100644 --- a/tests/jobs/test_removal_handler.py +++ b/tests/jobs/test_removal_handler.py @@ -52,3 +52,23 @@ 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() 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_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/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..1ceee232 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,76 @@ 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" diff --git a/tests/settings/test_instances.py b/tests/settings/test_instances.py index 2f150635..d320a19e 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", @@ -61,3 +85,117 @@ 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 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/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() From 701f67f05310b46540bf184c69b6981fc5a26bd3 Mon Sep 17 00:00:00 2001 From: JR Hager Date: Thu, 9 Jul 2026 13:02:11 -0600 Subject: [PATCH 13/26] Support qBittorrent 5.2 API-key authentication (closes #353) qBittorrent 5.2 added stateless API-key auth (Authorization: Bearer), which lets users avoid storing username/password credentials. This adds an optional `api_key` field to qBittorrent clients and makes it the recommended auth path; username/password remain as a legacy fallback for qBit < 5.2. - QbitClient accepts `api_key`; a new `_auth_kwargs()` helper returns the Bearer header in key mode or the SID cookie in password mode, used at every authenticated request site. - refresh_cookie() and check_qbit_reachability() branch for key mode: login is skipped (qBit rejects /auth/login under key auth) and reachability probes /app/version with the Bearer header instead. A bad key or a <5.2 server both surface a clear tip and degrade instead of crashing. - If both api_key and username/password are set, the key wins (mirrors qBit), logged once at init. - Redact `Authorization` in sanitize_kwargs so the token never leaks in DEBUG logs. - Docs (config example, README) present api_key as recommended, creds as legacy. Tests cover header-vs-cookie selection, login skip, precedence, and the 403 degrade path. --- README.md | 10 +- config/config_example.yaml | 5 +- src/settings/_download_clients_qbit.py | 138 +++++++++++---- src/utils/common.py | 9 +- tests/settings/test_download_clients_qbit.py | 166 +++++++++++++++++++ tests/utils/test_common.py | 11 ++ 6 files changed, 300 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index ec1d8171..d4611e8a 100644 --- a/README.md +++ b/README.md @@ -288,8 +288,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" @@ -704,8 +705,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 8afc5ebd..2bbf8094 100644 --- a/config/config_example.yaml +++ b/config/config_example.yaml @@ -71,8 +71,9 @@ 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: diff --git a/src/settings/_download_clients_qbit.py b/src/settings/_download_clients_qbit.py index dc1d8b87..8d6896f1 100644 --- a/src/settings/_download_clients_qbit.py +++ b/src/settings/_download_clients_qbit.py @@ -59,6 +59,7 @@ def __init__( password: str = None, name: str = None, timeout: int | None = None, + api_key: str = None, ): self.settings = settings self._timeout = timeout @@ -72,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( @@ -79,6 +82,11 @@ 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 @@ -94,8 +102,27 @@ def _remove_none_attributes(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." @@ -155,7 +182,7 @@ async def fetch_version(self): endpoint, self.settings, timeout=self.timeout, - cookies=self.cookie, + **self._auth_kwargs(), ) self.version = response.text[1:] # Remove the '_v' prefix logger.debug( @@ -177,13 +204,38 @@ async def validate_version(self): "[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, timeout=self.timeout, 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: @@ -195,7 +247,7 @@ async def create_tag(self, tag: str): self.settings, timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def create_required_tags(self): @@ -220,7 +272,7 @@ async def set_unwanted_folder(self): endpoint, self.settings, timeout=self.timeout, - cookies=self.cookie, + **self._auth_kwargs(), ) qbit_settings = response.json() @@ -235,7 +287,7 @@ async def set_unwanted_folder(self): self.settings, timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def check_qbit_reachability(self): @@ -244,24 +296,44 @@ async def check_qbit_reachability(self): 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"} - response = await make_request( - "post", - endpoint, - self.settings, - timeout=self.timeout, - 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?" + 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 @@ -289,14 +361,15 @@ async def check_connected(self): self.api_url + "/sync/maindata", self.settings, timeout=self.timeout, - cookies=self.cookie, + **self._auth_kwargs(), ) ).json() )["server_state"]["connection_status"] - except Exception: + except Exception as e: logger.warning( - ">>> %s: Failed to reach /sync/maindata. Treating as disconnected.", + ">>> %s: Failed to reach /sync/maindata (%s). Treating as disconnected.", self.name, + e, ) return False if qbit_connection_status == "disconnected": @@ -314,6 +387,7 @@ async def setup(self): # Fetch version and validate it await self.fetch_version() await self.validate_version() + self.log_auth_version_guidance() await self.create_required_tags() await self.set_unwanted_folder() @@ -411,7 +485,7 @@ async def set_tag(self, tags, hashes): self.settings, timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def fetch_download_progress(self, download_id): @@ -435,7 +509,7 @@ async def get_qbit_items(self, hashes: list[str] | str | None = None) -> list[di settings=self.settings, timeout=self.timeout, params=None, # Retrieve all torrents - cookies=self.cookie, + **self._auth_kwargs(), ) all_items = response.json() @@ -458,7 +532,7 @@ async def get_torrent_properties(self, qbit_hash): self.settings, timeout=self.timeout, params=params, - cookies=self.cookie, + **self._auth_kwargs(), ) return response.json() @@ -471,7 +545,7 @@ async def get_torrent_files(self, download_id): settings=self.settings, timeout=self.timeout, params={"hash": download_id.lower()}, - cookies=self.cookie, + **self._auth_kwargs(), ) return response.json() @@ -490,7 +564,7 @@ async def set_torrent_file_priority(self, download_id, file_id, priority=0): self.settings, timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) async def set_bandwidth_usage(self): @@ -501,7 +575,7 @@ async def set_bandwidth_usage(self): endpoint=self.api_url + "/transfer/info", settings=self.settings, timeout=self.timeout, - cookies=self.cookie, + **self._auth_kwargs(), ) records = extract_json_from_response(response) limit = records["dl_rate_limit"] @@ -539,5 +613,5 @@ async def remove_download(self, download_hash: str, delete_files: bool = True): self.settings, timeout=self.timeout, data=data, - cookies=self.cookie, + **self._auth_kwargs(), ) diff --git a/src/utils/common.py b/src/utils/common.py index 4bcab1df..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**]" diff --git a/tests/settings/test_download_clients_qbit.py b/tests/settings/test_download_clients_qbit.py index 1ceee232..8f51d3ff 100644 --- a/tests/settings/test_download_clients_qbit.py +++ b/tests/settings/test_download_clients_qbit.py @@ -111,3 +111,169 @@ async def test_setup_bad_password_marks_definitive(): 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/utils/test_common.py b/tests/utils/test_common.py index 9b8afc0a..5180629f 100644 --- a/tests/utils/test_common.py +++ b/tests/utils/test_common.py @@ -45,3 +45,14 @@ async def test_make_request_allows_explicit_timeout_override(): 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) From 58d4d7ce4607cdfcfc7160e9e1c4e5c063788d8e Mon Sep 17 00:00:00 2001 From: JR Hager Date: Sun, 12 Jul 2026 17:29:56 -0600 Subject: [PATCH 14/26] Surface actionable errors for malformed env YAML --- src/settings/_user_config.py | 6 +++++- tests/settings/test_user_config_from_env.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/settings/_user_config.py b/src/settings/_user_config.py index 7ba74f3b..93526acf 100644 --- a/src/settings/_user_config.py +++ b/src/settings/_user_config.py @@ -93,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/tests/settings/test_user_config_from_env.py b/tests/settings/test_user_config_from_env.py index b35f345f..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 @@ -137,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 From 105584a2b150e116df35dc99b729feb8068fbfe4 Mon Sep 17 00:00:00 2001 From: JR Hager Date: Sun, 12 Jul 2026 17:33:32 -0600 Subject: [PATCH 15/26] Skip stale queue items during unmonitored checks --- src/jobs/remove_unmonitored.py | 22 +++++++++++++++- tests/jobs/test_remove_unmonitored.py | 38 ++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) 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/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 From 1a359c453c4923bbd8147d5918b9f32ab730792b Mon Sep 17 00:00:00 2001 From: Ryan Christensen Date: Thu, 16 Jul 2026 14:24:43 -0500 Subject: [PATCH 16/26] Fix metadata missing detection in full queue --- src/jobs/remove_metadata_missing.py | 16 +- tests/jobs/test_remove_metadata_missing.py | 238 ++++++++++++++++++++- 2 files changed, 248 insertions(+), 6 deletions(-) diff --git a/src/jobs/remove_metadata_missing.py b/src/jobs/remove_metadata_missing.py index 07c1150e..7b4503f8 100644 --- a/src/jobs/remove_metadata_missing.py +++ b/src/jobs/remove_metadata_missing.py @@ -2,7 +2,7 @@ class RemoveMetadataMissing(RemovalJob): - queue_scope = "normal" + queue_scope = "full" blocklist = True async def _find_affected_items(self): @@ -14,12 +14,18 @@ async def _find_affected_items(self): # 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): - seen = {id(item) for item in affected_items} - for item in self.queue_manager.filter_missing_size(self.queue): - if id(item) not in seen: + 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(id(item)) + seen.add(item_identity) return affected_items diff --git a/tests/jobs/test_remove_metadata_missing.py b/tests/jobs/test_remove_metadata_missing.py index 1ff2863f..2447387c 100644 --- a/tests/jobs/test_remove_metadata_missing.py +++ b/tests/jobs/test_remove_metadata_missing.py @@ -1,4 +1,5 @@ -from unittest.mock import MagicMock +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock import pytest @@ -6,6 +7,67 @@ 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( @@ -90,6 +152,7 @@ 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) @@ -187,6 +250,179 @@ async def test_find_affected_items_via_missing_size( # 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 + ) + + +@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 + ) + + +@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 + ) + 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 + ) + + +@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 + ) + 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 + ) + assert removal_job.arr.tracker.deleted == [normal_size_zero["downloadId"]] From d8fe715986dca9f16df7c75ffb25cc51a07d8e11 Mon Sep 17 00:00:00 2001 From: ManiMatter <124743318+ManiMatter@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:11:38 +0200 Subject: [PATCH 17/26] Update README with version hints and breaking changes Added hints for checking the correct README version and noted breaking changes in Decluttar V2. --- README.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1825f0fe..b375f91a 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) From 304f8c41b316a2bf628f945fdd3eb3182ff407e0 Mon Sep 17 00:00:00 2001 From: ManiMatter <124743318+ManiMatter@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:19:57 +0200 Subject: [PATCH 18/26] Clarify README version in bug report template Updated the bug report template to clarify README version references. --- .github/ISSUE_TEMPLATE/bug_report.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 2c005d1c..1c57aa1c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -9,7 +9,11 @@ body: 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](https://github.com/ManiMatter/decluttarr#dependencies--hints--faq), especially the **Dependencies & Hints & FAQ** section + - 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) From c80492c60bbd534ed5ef085a261f75cf6641c94b Mon Sep 17 00:00:00 2001 From: Sportarr Date: Tue, 4 Aug 2026 11:23:13 -0400 Subject: [PATCH 19/26] Add Sportarr support Adds Sportarr (https://github.com/Sportarr/Sportarr), a sports event manager exposing a Sonarr-v3-compatible API, as a supported instance type. It rides the sonarr-shaped constants (api/v3, series/episode keys, EpisodeSearch/RefreshSeries commands, includeUnknownSeriesItems) and joins sonarr in every per-type conditional: instance loading, refresh plumbing, deletion watchers, search logging, and startup rejoin. README and config example gain SPORTARR/sportarr blocks. Full test suite passes (292) and ruff reports no new findings on the touched files. --- README.md | 6 +++++- config/config_example.yaml | 3 +++ src/deletion_handler/deletion_handler.py | 1 + src/jobs/search_handler.py | 2 +- src/settings/_constants.py | 7 +++++++ src/settings/_instances.py | 4 ++-- src/settings/_user_config.py | 2 +- src/utils/startup.py | 2 +- 8 files changed, 21 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b375f91a..e073727b 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,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: @@ -276,6 +276,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" diff --git a/config/config_example.yaml b/config/config_example.yaml index 2bbf8094..01277f82 100644 --- a/config/config_example.yaml +++ b/config/config_example.yaml @@ -55,6 +55,9 @@ instances: - 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" diff --git a/src/deletion_handler/deletion_handler.py b/src/deletion_handler/deletion_handler.py index aa1511f3..8b180f67 100644 --- a/src/deletion_handler/deletion_handler.py +++ b/src/deletion_handler/deletion_handler.py @@ -122,6 +122,7 @@ async def get_folders_to_watch_for_arr(self, arr): folders_to_watch = [] if arr.arr_type not in ( "sonarr", + "sportarr", "radarr", ): # only working for sonarr / radarr for now return folders_to_watch 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/_instances.py b/src/settings/_instances.py index 99bd772b..ee682c1d 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -91,7 +91,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( @@ -174,7 +174,7 @@ def __init__(self, settings, arr_type: str, base_url: str, api_key: str, timeout 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) diff --git a/src/settings/_user_config.py b/src/settings/_user_config.py index 93526acf..a1583b20 100644 --- a/src/settings/_user_config.py +++ b/src/settings/_user_config.py @@ -41,7 +41,7 @@ "SEARCH_UNMET_CUTOFF", "SEARCH_MISSING", ], - "instances": ["SONARR", "RADARR", "READARR", "LIDARR", "WHISPARR"], + "instances": ["SONARR", "SPORTARR", "RADARR", "READARR", "LIDARR", "WHISPARR"], "download_clients": ["QBITTORRENT"], } diff --git a/src/utils/startup.py b/src/utils/startup.py index 535ab331..e61eef85 100644 --- a/src/utils/startup.py +++ b/src/utils/startup.py @@ -133,7 +133,7 @@ async def retry_degraded_instances(settings, watch_manager=None): ) if ( watch_manager - and getattr(unit, "arr_type", None) in ("sonarr", "radarr") + and getattr(unit, "arr_type", None) in ("sonarr", "sportarr", "radarr") and settings.jobs.detect_deletions.enabled ): await watch_manager.setup_for_arr(unit) From 79851e6248da69a2f9388a2b6a1b70ff6c8ea00e Mon Sep 17 00:00:00 2001 From: Sportarr Date: Tue, 4 Aug 2026 14:50:03 -0400 Subject: [PATCH 20/26] Detect Sportarr via its identity marker instead of appName --- src/settings/_instances.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/settings/_instances.py b/src/settings/_instances.py index ee682c1d..6081b2c0 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -219,6 +219,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( From 57f901f1b81d1fb298d0471f50c11f75064d2e4d Mon Sep 17 00:00:00 2001 From: Matthew Crenshaw Date: Fri, 28 Aug 2026 12:42:50 -0400 Subject: [PATCH 21/26] Add remove_from_queue tracker handling mode Private-tracker torrents currently have no handling option that clears the *arr queue without also deleting the download. "remove" passes removeFromClient=true, which stops seeding and can trigger a hit-and-run penalty; "obsolete_tag" only tags the torrent and leaves the queue entry in place indefinitely, so the *arr keeps waiting on a download that will never import. remove_from_queue is "remove" minus the client deletion: the queue entry is cleared and blocklisted per the job's existing blocklist policy, while the torrent stays in the client for an external tool (e.g. qbit_manage) to retire on the tracker's terms. remove_queue_item() gains a remove_from_client keyword, defaulting to True so existing behaviour is unchanged. --- README.md | 3 +- config/config_example.yaml | 4 +- src/jobs/removal_handler.py | 25 +++++++-- src/settings/_general.py | 2 +- src/settings/_instances.py | 13 ++++- tests/jobs/test_removal_handler.py | 64 ++++++++++++++++++++++ tests/jobs/test_remove_metadata_missing.py | 12 ++-- tests/settings/test_general.py | 7 +++ tests/settings/test_instances.py | 40 ++++++++++++++ 9 files changed, 153 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index b375f91a..7737ed0e 100644 --- a/README.md +++ b/README.md @@ -439,8 +439,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) diff --git a/config/config_example.yaml b/config/config_example.yaml index 2bbf8094..6729740e 100644 --- a/config/config_example.yaml +++ b/config/config_example.yaml @@ -6,8 +6,8 @@ general: # ignored_download_clients: ["emulerr"] # ssl_verification: false # Optional: Defaults to true # request_timeout: 15 # Optional: Request timeout in seconds. Defaults to 15. Can be overridden per instance. - # private_tracker_handling: "obsolete_tag" # remove, skip, obsolete_tag. Optional. Default: remove - # public_tracker_handling: "remove" # remove, skip, obsolete_tag. Optional. Default: remove + # 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" diff --git a/src/jobs/removal_handler.py b/src/jobs/removal_handler.py index 122c1930..aeb25132 100644 --- a/src/jobs/removal_handler.py +++ b/src/jobs/removal_handler.py @@ -21,6 +21,13 @@ 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) @@ -31,13 +38,23 @@ 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( diff --git a/src/settings/_general.py b/src/settings/_general.py index dad405e4..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: diff --git a/src/settings/_instances.py b/src/settings/_instances.py index 99bd772b..f2609f40 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -341,7 +341,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. @@ -350,17 +352,22 @@ 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( diff --git a/tests/jobs/test_removal_handler.py b/tests/jobs/test_removal_handler.py index 0827ede5..a9dc33ab 100644 --- a/tests/jobs/test_removal_handler.py +++ b/tests/jobs/test_removal_handler.py @@ -72,3 +72,67 @@ async def test_tag_as_obsolete_skips_not_ready_qbit(): 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_remove_metadata_missing.py b/tests/jobs/test_remove_metadata_missing.py index 2447387c..ff7bdb48 100644 --- a/tests/jobs/test_remove_metadata_missing.py +++ b/tests/jobs/test_remove_metadata_missing.py @@ -274,7 +274,7 @@ async def test_run_detects_metadata_stuck_item_present_only_in_full_queue(): queue_scope="full" ) removal_job.arr.remove_queue_item.assert_awaited_once_with( - queue_id=full_only_item["id"], blocklist=True + queue_id=full_only_item["id"], blocklist=True, remove_from_client=True ) @@ -293,7 +293,7 @@ async def test_run_still_detects_metadata_stuck_item_in_normal_queue(): assert removed == 1 removal_job.arr.remove_queue_item.assert_awaited_once_with( - queue_id=normal_item["id"], blocklist=True + queue_id=normal_item["id"], blocklist=True, remove_from_client=True ) @@ -348,7 +348,7 @@ async def test_run_keeps_ignored_client_and_protected_tag_downloads(): assert removed == 1 removal_job.arr.remove_queue_item.assert_awaited_once_with( - queue_id=removable_item["id"], blocklist=True + queue_id=removable_item["id"], blocklist=True, remove_from_client=True ) assert removal_job.arr.tracker.deleted == [removable_item["downloadId"]] @@ -380,7 +380,7 @@ async def test_run_preserves_strikes_and_blocklist_for_full_only_item(): == 2 ) removal_job.arr.remove_queue_item.assert_awaited_once_with( - queue_id=full_only_item["id"], blocklist=True + queue_id=full_only_item["id"], blocklist=True, remove_from_client=True ) @@ -395,7 +395,7 @@ async def test_run_removes_duplicate_queue_rows_only_once(): assert removed == 1 removal_job.arr.remove_queue_item.assert_awaited_once_with( - queue_id=first_row["id"], blocklist=True + queue_id=first_row["id"], blocklist=True, remove_from_client=True ) assert removal_job.arr.tracker.deleted == [first_row["downloadId"]] @@ -423,6 +423,6 @@ async def test_missing_size_opt_in_remains_limited_to_normal_queue(): assert removed == 1 removal_job.arr.remove_queue_item.assert_awaited_once_with( - queue_id=normal_size_zero["id"], blocklist=True + 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/settings/test_general.py b/tests/settings/test_general.py index 52aa0753..b7789eb7 100644 --- a/tests/settings/test_general.py +++ b/tests/settings/test_general.py @@ -11,3 +11,10 @@ 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 d320a19e..493dea35 100644 --- a/tests/settings/test_instances.py +++ b/tests/settings/test_instances.py @@ -199,3 +199,43 @@ async def test_setup_wrong_arr_type_and_old_version_still_pass(): 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 From a460f936707dc96cac1ffb429062383b294b0db3 Mon Sep 17 00:00:00 2001 From: Nathan Guenther Date: Fri, 28 Aug 2026 15:45:18 -0400 Subject: [PATCH 22/26] Fix bool/dict job configs losing built-in defaults PR #333 added JobParams.__bool__ (truthiness = enabled). Jobs._set_job_settings uses `if job:` to decide whether to reuse the pre-built defaults object for a job, and those defaults objects start with enabled=False, so the check is now always false. Every job configured as a bare `true` (or as a dict) is replaced by a fresh JobParams carrying only the keys the user typed: remove_failed_imports: true -> no message_patterns -> AttributeError ('JobParams' object has no attribute 'message_patterns') remove_stalled: true -> no max_strikes -> StrikesHandler skipped, remove_slow: {min_speed: 25} i.e. removal on first sight instead of after 3 strikes Check for `is not None` instead, which is what the code meant before __bool__ existed. Adds regression tests for bool true/false and dict merging. Co-Authored-By: Claude Opus 5 --- src/settings/_jobs.py | 4 ++-- tests/settings/test_jobs.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/settings/_jobs.py b/src/settings/_jobs.py index c14d678b..71d39abb 100644 --- a/src/settings/_jobs.py +++ b/src/settings/_jobs.py @@ -127,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/tests/settings/test_jobs.py b/tests/settings/test_jobs.py index 9d4c6144..d8d1927c 100644 --- a/tests/settings/test_jobs.py +++ b/tests/settings/test_jobs.py @@ -8,3 +8,38 @@ def test_job_params_truthiness_is_false_by_default(): 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 From 72dd989ea21c5b242d11b07e171644189c164785 Mon Sep 17 00:00:00 2001 From: qoole <2862661+qoole@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:16:22 +0100 Subject: [PATCH 23/26] fix: re-arm removals for downloads that never left the queue A removal that does not take effect permanently disables every future removal of that download. tracker.deleted gates every removal (removal_handler.py:18) and is appended unconditionally (line 32) without checking whether the removal worked, yet it is only ever cleared by Tracker.reset() when the *arr queue is completely empty. A download that cannot be removed keeps the queue non-empty, so the entry is never cleared and that download stays suppressed for the life of the process. Observed on v2.1.0 as a stuck torrent reaching 74 strikes against max_strikes 5 over ~19 hours with zero removals and zero blocklist entries, and no log line explaining it. Prune the tracker at the start of each run using the full queue that _queue_has_items() already fetches: if a download recorded as deleted is still in the queue, the removal did not take effect, so it becomes eligible again. Entries for downloads that really are gone are kept, so the cross-cycle reader in strikes_handler stays correct. obsolete_tag handling deliberately leaves the download in the queue, so queue presence does not disprove it. Tagged ids are recorded separately in Tracker.obsolete_tagged and exempted from the prune; without this an obsolete-tagged torrent is re-tagged on every cycle. Co-Authored-By: Claude Opus 5 (1M context) --- src/job_manager.py | 36 +++++++++ src/jobs/removal_handler.py | 1 + src/settings/_instances.py | 2 + tests/jobs/test_job_manager.py | 136 +++++++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+) diff --git a/src/job_manager.py b/src/job_manager.py index 223f4aa2..564f9ba5 100644 --- a/src/job_manager.py +++ b/src/job_manager.py @@ -136,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, diff --git a/src/jobs/removal_handler.py b/src/jobs/removal_handler.py index 122c1930..d64077ab 100644 --- a/src/jobs/removal_handler.py +++ b/src/jobs/removal_handler.py @@ -23,6 +23,7 @@ async def remove_downloads(self, affected_downloads, blocklist): await self._remove_download(affected_download, download_id, blocklist) 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: diff --git a/src/settings/_instances.py b/src/settings/_instances.py index 99bd772b..56046d2e 100644 --- a/src/settings/_instances.py +++ b/src/settings/_instances.py @@ -29,6 +29,7 @@ def __init__(self): self.defective = {} self.download_progress = {} self.deleted = [] + self.obsolete_tagged = [] self.extension_checked = [] def reset(self) -> None: @@ -38,6 +39,7 @@ def reset(self) -> None: self.defective, self.download_progress, self.deleted, + self.obsolete_tagged, self.extension_checked, ): attr.clear() diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py index 7a1d774f..f5101a0f 100644 --- a/tests/jobs/test_job_manager.py +++ b/tests/jobs/test_job_manager.py @@ -4,6 +4,9 @@ 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 @@ -75,3 +78,136 @@ async def test_run_download_client_jobs_handles_connection_check_request_errors( 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) + + +@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() From c2e671e503da90b481c5a1f2ff675e514d134383 Mon Sep 17 00:00:00 2001 From: keez97 Date: Wed, 23 Sep 2026 12:45:40 +0400 Subject: [PATCH 24/26] Fix remove_orphans deleting active downloads - compare the full and normal queue by item id instead of whole records; sizeleft/timeleft of an active download change between the two fetches, so any progressing download was flagged as an orphan and deleted (#322) - regression test --- src/utils/queue_manager.py | 6 +++++- tests/utils/test_queue_manager.py | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/utils/queue_manager.py b/src/utils/queue_manager.py index 4aa5a4d4..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: 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] From d855964c2bed9c1e70511a30291afa438a53287b Mon Sep 17 00:00:00 2001 From: ManiMatter <124743318+ManiMatter@users.noreply.github.com> Date: Sun, 27 Sep 2026 07:56:01 +0200 Subject: [PATCH 25/26] Add Sportarr section to README --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index e073727b..22ad8913 100644 --- a/README.md +++ b/README.md @@ -57,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) @@ -702,6 +703,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) --- From c71571bef11cabd654dcb277ab763eeb677abc12 Mon Sep 17 00:00:00 2001 From: juan Date: Tue, 29 Sep 2026 13:57:39 -0700 Subject: [PATCH 26/26] test: expect remove_from_client kwarg in re-arm removal test #383 added this assertion before remove_queue_item gained the remove_from_client keyword, so it fails once both are on dev. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rK7uZPvqGrze3sxdhVnGw --- tests/jobs/test_job_manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py index f5101a0f..e7dfbfb4 100644 --- a/tests/jobs/test_job_manager.py +++ b/tests/jobs/test_job_manager.py @@ -151,7 +151,9 @@ async def test_forgotten_download_is_removed_again_on_the_next_run(): ) # Assert - manager.arr.remove_queue_item.assert_awaited_once_with(queue_id=42, blocklist=True) + manager.arr.remove_queue_item.assert_awaited_once_with( + queue_id=42, blocklist=True, remove_from_client=True + ) @pytest.mark.asyncio