From 6bab9989ab70b65debbd943e95ef93d78dc9976b Mon Sep 17 00:00:00 2001 From: CaliBrain Date: Tue, 14 Jul 2026 16:21:31 -0400 Subject: [PATCH] fix: fetch tracker .torrent links once per add and surface fetch failures (#1115) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes Prowlarr torrent downloads that fail with `Could not determine torrent hash from URL` when the result has no magnet link and no infohash (e.g. MyAnonaMouse), where fetching the .torrent from Prowlarr's proxy download link is the only path. Two problems compounded here: 1. **Every add attempt fetched the download link twice.** `find_existing()` prefetched the .torrent to compute a dedup hash, discarded the result, and `add_download()` fetched the same URL again seconds later. Private tracker links behind Prowlarr's proxy can be slow, rate-limited, or effectively single-use, so the second hit could fail even when the link itself was valid — which is why the reporter's manual fetch of the same URL succeeded. 2. **The real failure reason was invisible.** When the fetch failed (e.g. Prowlarr returning HTTP 500 because the tracker rejected the request — see the 2026-07-07 MAM report on #476, which turned out to be a MAM IP-settings problem), the reason was logged at DEBUG only and the user saw the misleading generic hash error. ## What changed - `extract_torrent_info()` now reuses a recent successful fetch of the same URL (short-TTL in-memory cache, successes only), so one add attempt hits the tracker download link exactly once across `find_existing()` + `add_download()`. All four torrent clients (qBittorrent, Deluge, Transmission, rTorrent) share this path and benefit. Failures are never cached, so retries refetch. - `TorrentInfo` gains a `fetch_error` field. qBittorrent and rTorrent append it to the hash error (`... (torrent file fetch failed: 500 Server Error ...)`), Deluge to its "Failed to fetch torrent file" error. The enriched message still contains the exact substring the #1109 expired-link refresh hook matches on, so the refresh-and-retry path keeps working. - Torrent fetch failures are logged at WARNING instead of DEBUG, so non-debug logs show the cause. ## Validation - `uv run pytest tests/prowlarr tests/download -q` — 498 passed - `uv run pytest tests/newznab tests/audiobookbay -q` — 147 passed - `uv run ruff check` / `ruff format --check` on all changed files - New tests: fetch-cache reuse, failure-not-cached + reason capture, expected-hash fallback on failed/hashless fetches, TTL expiry, magnet-redirect reuse, and the enriched qBittorrent error message. Fixes #1111 --- shelfmark/download/clients/deluge.py | 5 +- shelfmark/download/clients/qbittorrent.py | 5 +- shelfmark/download/clients/rtorrent.py | 5 +- shelfmark/download/clients/torrent_utils.py | 82 ++++++++++-- tests/conftest.py | 10 ++ tests/prowlarr/test_handler.py | 15 ++- tests/prowlarr/test_qbittorrent_client.py | 55 +++++++++ tests/prowlarr/test_torrent_utils.py | 130 ++++++++++++++++++++ 8 files changed, 285 insertions(+), 22 deletions(-) diff --git a/shelfmark/download/clients/deluge.py b/shelfmark/download/clients/deluge.py index cbaf3399..6b0620d5 100644 --- a/shelfmark/download/clients/deluge.py +++ b/shelfmark/download/clients/deluge.py @@ -277,7 +277,10 @@ class DelugeClient(DownloadClient): torrent_info = extract_torrent_info(url, expected_hash=expected_hash) if not torrent_info.is_magnet and not torrent_info.torrent_data: - _raise_runtime_error("Failed to fetch torrent file") + message = "Failed to fetch torrent file" + if torrent_info.fetch_error: + message = f"{message}: {torrent_info.fetch_error}" + _raise_runtime_error(message) options: dict[str, Any] = {} if self._download_dir: diff --git a/shelfmark/download/clients/qbittorrent.py b/shelfmark/download/clients/qbittorrent.py index ba3eaaf3..904afd7e 100644 --- a/shelfmark/download/clients/qbittorrent.py +++ b/shelfmark/download/clients/qbittorrent.py @@ -513,7 +513,10 @@ class QBittorrentClient(DownloadClient): # watching for the new torrent to appear. expected_hash = self._discover_added_torrent_hash(name, category, known_hashes) if not expected_hash: - _raise_runtime_error("Could not determine torrent hash from URL") + message = "Could not determine torrent hash from URL" + if torrent_info.fetch_error: + message = f"{message} (torrent file fetch failed: {torrent_info.fetch_error})" + _raise_runtime_error(message) # Some qBittorrent-compatible clients return HTTP 200 with an empty body # instead of qBittorrent's literal "Ok." response. Prefer verifying that diff --git a/shelfmark/download/clients/rtorrent.py b/shelfmark/download/clients/rtorrent.py index 5f2c0d4f..457376ab 100644 --- a/shelfmark/download/clients/rtorrent.py +++ b/shelfmark/download/clients/rtorrent.py @@ -206,7 +206,10 @@ class RTorrentClient(DownloadClient): # watching for the new download to appear. torrent_hash = self._discover_added_torrent_hash(name, label, known_hashes) if not torrent_hash: - _raise_runtime_error("Could not determine torrent hash from URL") + message = "Could not determine torrent hash from URL" + if torrent_info.fetch_error: + message = f"{message} (torrent file fetch failed: {torrent_info.fetch_error})" + _raise_runtime_error(message) logger.debug("Added torrent to rTorrent: %s", torrent_hash) diff --git a/shelfmark/download/clients/torrent_utils.py b/shelfmark/download/clients/torrent_utils.py index 0449f7ce..1e116d7e 100644 --- a/shelfmark/download/clients/torrent_utils.py +++ b/shelfmark/download/clients/torrent_utils.py @@ -5,8 +5,10 @@ from __future__ import annotations import base64 import hashlib import re +import time from binascii import Error as BinasciiError from dataclasses import dataclass +from threading import Lock from urllib.parse import ParseResult, parse_qs, urljoin, urlparse import requests @@ -36,6 +38,15 @@ _TORRENT_FETCH_ERRORS = ( _TORRENT_PARSE_ERRORS = (IndexError, KeyError, TypeError, ValueError) _TRUSTED_TORRENT_FETCH_URL_CONFIG_KEYS = ("PROWLARR_URL", "NEWZNAB_URL") +# Successful torrent fetches are reused for a short window so one add attempt +# hits the download link only once. Tracker download links (e.g. private +# trackers behind Prowlarr's proxy) can be slow, rate-limited, or single-use, +# and both find_existing() and add_download() resolve the same URL (#1111). +_TORRENT_FETCH_CACHE_TTL_SECONDS = 120.0 +_TORRENT_FETCH_CACHE_MAX_ENTRIES = 8 +_torrent_fetch_cache_lock = Lock() +_torrent_fetch_cache: dict[str, tuple[float, TorrentInfo]] = {} + type BencodeValue = dict[str | bytes, BencodeValue] | list[BencodeValue] | int | bytes | str @@ -55,6 +66,9 @@ class TorrentInfo: magnet_url: str | None = None """The actual magnet URL, if available.""" + fetch_error: str | None = None + """Why fetching the .torrent URL failed, or None if it succeeded/was skipped.""" + def with_info_hash(self, info_hash: str | None) -> TorrentInfo: """Return a copy with the info_hash replaced when provided.""" if info_hash: @@ -63,6 +77,7 @@ class TorrentInfo: torrent_data=self.torrent_data, is_magnet=self.is_magnet, magnet_url=self.magnet_url, + fetch_error=self.fetch_error, ) return self @@ -97,6 +112,48 @@ def extract_torrent_info( if not fetch_torrent: return TorrentInfo(info_hash=expected_hash, torrent_data=None, is_magnet=False) + info = _get_cached_torrent_fetch(url) + if info is None: + info = _fetch_torrent_info(url) + if info.fetch_error is None: + _store_cached_torrent_fetch(url, info) + + return info.with_info_hash(info.info_hash or expected_hash) + + +def _get_cached_torrent_fetch(url: str) -> TorrentInfo | None: + with _torrent_fetch_cache_lock: + entry = _torrent_fetch_cache.get(url) + if entry is None: + return None + fetched_at, info = entry + if time.monotonic() - fetched_at > _TORRENT_FETCH_CACHE_TTL_SECONDS: + del _torrent_fetch_cache[url] + return None + logger.debug("Reusing recently fetched torrent data for: %s...", url[:80]) + return info + + +def _store_cached_torrent_fetch(url: str, info: TorrentInfo) -> None: + with _torrent_fetch_cache_lock: + _torrent_fetch_cache[url] = (time.monotonic(), info) + while len(_torrent_fetch_cache) > _TORRENT_FETCH_CACHE_MAX_ENTRIES: + oldest_url = min(_torrent_fetch_cache, key=lambda key: _torrent_fetch_cache[key][0]) + del _torrent_fetch_cache[oldest_url] + + +def clear_torrent_fetch_cache() -> None: + """Drop all cached torrent fetches (used by tests).""" + with _torrent_fetch_cache_lock: + _torrent_fetch_cache.clear() + + +def _fetch_torrent_info(url: str) -> TorrentInfo: + """Fetch a .torrent URL and parse out the info_hash and raw torrent data. + + On failure, the returned TorrentInfo carries the reason in `fetch_error` + so callers can surface it instead of a generic hash error. + """ # A release source can legitimately hand us a download URL on a different # origin than the configured Prowlarr/Newznab endpoint (e.g. a direct # tracker link, or Prowlarr reached through a separate proxy), and a trusted @@ -145,18 +202,20 @@ def extract_torrent_info( redirect_url = resolve_url(current_url, resp.headers.get("Location", "")) if redirect_url.startswith("magnet:"): logger.debug("Download URL redirected to magnet link") - info_hash = extract_hash_from_magnet(redirect_url) - if not info_hash and expected_hash: - info_hash = expected_hash return TorrentInfo( - info_hash=info_hash, + info_hash=extract_hash_from_magnet(redirect_url), torrent_data=None, is_magnet=True, magnet_url=redirect_url, ) if redirects_remaining <= 0: - logger.debug("Too many redirects fetching torrent file: %s...", url[:80]) - return TorrentInfo(info_hash=expected_hash, torrent_data=None, is_magnet=False) + logger.warning("Too many redirects fetching torrent file: %s...", url[:80]) + return TorrentInfo( + info_hash=None, + torrent_data=None, + is_magnet=False, + fetch_error="too many redirects", + ) redirects_remaining -= 1 logger.debug("Following redirect to: %s...", redirect_url[:80]) current_url = redirect_url @@ -170,25 +229,22 @@ def extract_torrent_info( text_content = torrent_data.decode("utf-8", errors="ignore").strip() if text_content.startswith("magnet:"): logger.debug("Download URL returned magnet link as response body") - info_hash = extract_hash_from_magnet(text_content) - if not info_hash and expected_hash: - info_hash = expected_hash return TorrentInfo( - info_hash=info_hash, + info_hash=extract_hash_from_magnet(text_content), torrent_data=None, is_magnet=True, magnet_url=text_content, ) - info_hash = extract_info_hash_from_torrent(torrent_data) or expected_hash + info_hash = extract_info_hash_from_torrent(torrent_data) if info_hash: logger.debug("Extracted hash from torrent file: %s", info_hash) else: logger.warning("Could not extract hash from torrent file") return TorrentInfo(info_hash=info_hash, torrent_data=torrent_data, is_magnet=False) except _TORRENT_FETCH_ERRORS as e: - logger.debug("Could not fetch torrent file: %s", e) - return TorrentInfo(info_hash=expected_hash, torrent_data=None, is_magnet=False) + logger.warning("Could not fetch torrent file: %s", e) + return TorrentInfo(info_hash=None, torrent_data=None, is_magnet=False, fetch_error=str(e)) def _is_trusted_torrent_fetch_url(url: str) -> bool: diff --git a/tests/conftest.py b/tests/conftest.py index 2dcb1646..fee6198f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -29,6 +29,16 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import pytest +@pytest.fixture(autouse=True) +def _clear_torrent_fetch_cache(): + """Keep the shared torrent fetch cache from leaking between tests.""" + from shelfmark.download.clients.torrent_utils import clear_torrent_fetch_cache + + clear_torrent_fetch_cache() + yield + clear_torrent_fetch_cache() + + @pytest.fixture def sample_prowlarr_result(): """Sample Prowlarr API search result.""" diff --git a/tests/prowlarr/test_handler.py b/tests/prowlarr/test_handler.py index 56174805..2b99ca94 100644 --- a/tests/prowlarr/test_handler.py +++ b/tests/prowlarr/test_handler.py @@ -109,12 +109,15 @@ class TestProwlarrHandlerDownloadErrors: def test_download_fails_clearly_when_cache_miss_cannot_refresh(self): """Prowlarr retry URLs are not durable; cache misses must refresh by identity.""" - with patch( - "shelfmark.release_sources.prowlarr.handler.get_release", - return_value=None, - ), patch( - "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", - return_value=[], + with ( + patch( + "shelfmark.release_sources.prowlarr.handler.get_release", + return_value=None, + ), + patch( + "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", + return_value=[], + ), ): handler = ProwlarrHandler() task = DownloadTask( diff --git a/tests/prowlarr/test_qbittorrent_client.py b/tests/prowlarr/test_qbittorrent_client.py index fb098d9d..8d2fbb6a 100644 --- a/tests/prowlarr/test_qbittorrent_client.py +++ b/tests/prowlarr/test_qbittorrent_client.py @@ -903,6 +903,61 @@ class TestQBittorrentClientAddDownload: "http://tracker.example/download/book.torrent", "Test Download" ) + def test_add_download_error_includes_fetch_failure_reason(self, monkeypatch): + """Surface why the .torrent prefetch failed instead of only the hash error. + + Regression for #1111: a Prowlarr proxy fetch that fails (e.g. HTTP 500 + because the tracker rejected the request) was reported as a bare + "Could not determine torrent hash from URL", hiding the actual cause. + """ + config_values = { + "QBITTORRENT_URL": "http://localhost:8080", + "QBITTORRENT_USERNAME": "admin", + "QBITTORRENT_PASSWORD": "password", + "QBITTORRENT_CATEGORY": "books", + } + monkeypatch.setattr( + "shelfmark.download.clients.qbittorrent.config.get", + lambda key, default="": config_values.get(key, default), + ) + monkeypatch.setattr( + "shelfmark.download.clients.qbittorrent.time.sleep", lambda _seconds: None + ) + + mock_client_instance = MagicMock() + mock_client_instance.torrents_add.return_value = "Ok." + mock_client_instance._session.get.return_value = create_mock_session_response([]) + mock_client_class = MagicMock(return_value=mock_client_instance) + + with patch.dict("sys.modules", {"qbittorrentapi": MagicMock(Client=mock_client_class)}): + import importlib + + import shelfmark.download.clients.qbittorrent as qb_module + + importlib.reload(qb_module) + + with patch( + "shelfmark.download.clients.qbittorrent.extract_torrent_info", + autospec=True, + ) as mock_extract: + mock_extract.return_value = TorrentInfo( + info_hash=None, + torrent_data=None, + is_magnet=False, + magnet_url=None, + fetch_error="500 Server Error: Internal Server Error for url: http://prowlarr:9696/26/download", + ) + + client = qb_module.QBittorrentClient() + with pytest.raises(RuntimeError) as exc_info: + client.add_download( + "http://prowlarr:9696/26/download?apikey=key&link=token", + "Test Download", + ) + + assert "Could not determine torrent hash from URL" in str(exc_info.value) + assert "500 Server Error" in str(exc_info.value) + def test_add_download_creates_category(self, monkeypatch): """Test that add_download creates category if needed.""" config_values = { diff --git a/tests/prowlarr/test_torrent_utils.py b/tests/prowlarr/test_torrent_utils.py index ed565999..7a2332b8 100644 --- a/tests/prowlarr/test_torrent_utils.py +++ b/tests/prowlarr/test_torrent_utils.py @@ -668,3 +668,133 @@ class TestExtractHashFromMagnet: magnet = f"magnet:?xt=urn:btmh:{b32}&dn=test" result = extract_hash_from_magnet(magnet) assert result == digest.hex() + + +class TestTorrentFetchCache: + """Tests for reusing a fetched .torrent across find_existing/add_download (#1111).""" + + @staticmethod + def _valid_torrent(): + info_dict = { + b"name": b"book.txt", + b"length": 100, + b"piece length": 16384, + b"pieces": b"\x00" * 20, + } + torrent_data = bencode_encode({b"info": info_dict}) + info_hash = hashlib.sha1(bencode_encode(info_dict)).hexdigest().lower() + return torrent_data, info_hash + + def test_repeated_url_reuses_fetched_torrent_data(self, monkeypatch): + """The same download URL is fetched once per add attempt, not once per caller. + + find_existing() and add_download() both resolve the request URL; private + tracker links behind Prowlarr's proxy can be rate-limited or single-use, + so the second fetch must be served from the cache. + """ + torrent_data, info_hash = self._valid_torrent() + response = MagicMock(status_code=200, content=torrent_data) + response.raise_for_status = MagicMock() + mock_get = MagicMock(return_value=response) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + url = "https://prowlarr.example/26/download?apikey=secret&link=token" + first = extract_torrent_info(url, fetch_torrent=True) + second = extract_torrent_info(url, fetch_torrent=True) + + mock_get.assert_called_once() + assert first.info_hash == info_hash + assert second.info_hash == info_hash + assert second.torrent_data == torrent_data + + def test_failed_fetch_is_not_cached_and_records_reason(self, monkeypatch): + """Fetch failures are retried on the next call and expose the reason.""" + import requests as requests_module + + mock_get = MagicMock( + side_effect=requests_module.exceptions.HTTPError( + "500 Server Error: Internal Server Error for url: https://prowlarr.example/26/download" + ) + ) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + url = "https://prowlarr.example/26/download?apikey=secret&link=token" + first = extract_torrent_info(url, fetch_torrent=True) + second = extract_torrent_info(url, fetch_torrent=True) + + assert mock_get.call_count == 2 + assert first.fetch_error is not None + assert "500 Server Error" in first.fetch_error + assert first.info_hash is None + assert second.fetch_error is not None + + def test_fetch_failure_still_falls_back_to_expected_hash(self, monkeypatch): + """A known infohash keeps working when the prefetch fails.""" + import requests as requests_module + + mock_get = MagicMock(side_effect=requests_module.exceptions.ConnectionError("boom")) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + known_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0" + result = extract_torrent_info( + "https://tracker.example/download/book.torrent", + fetch_torrent=True, + expected_hash=known_hash, + ) + + assert result.info_hash == known_hash + assert result.fetch_error is not None + assert "boom" in result.fetch_error + + def test_expected_hash_applies_to_cached_hashless_result(self, monkeypatch): + """A cached fetch without a hash still honors a caller's expected_hash.""" + response = MagicMock(status_code=200, content=b"not a torrent") + response.raise_for_status = MagicMock() + mock_get = MagicMock(return_value=response) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + url = "https://tracker.example/download/book.torrent" + known_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0" + first = extract_torrent_info(url, fetch_torrent=True) + second = extract_torrent_info(url, fetch_torrent=True, expected_hash=known_hash) + + mock_get.assert_called_once() + assert first.info_hash is None + assert second.info_hash == known_hash + + def test_cache_expires_after_ttl(self, monkeypatch): + """Stale cache entries are refetched instead of reused.""" + torrent_data, _ = self._valid_torrent() + response = MagicMock(status_code=200, content=torrent_data) + response.raise_for_status = MagicMock() + mock_get = MagicMock(return_value=response) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + clock = {"now": 1000.0} + monkeypatch.setattr( + "shelfmark.download.clients.torrent_utils.time.monotonic", + lambda: clock["now"], + ) + + url = "https://tracker.example/download/book.torrent" + extract_torrent_info(url, fetch_torrent=True) + clock["now"] += 121.0 + extract_torrent_info(url, fetch_torrent=True) + + assert mock_get.call_count == 2 + + def test_magnet_redirect_result_is_reused(self, monkeypatch): + """A URL that redirects to a magnet link is also only resolved once.""" + magnet = "magnet:?xt=urn:btih:3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0&dn=test" + response = MagicMock(status_code=302, headers={"Location": magnet}) + mock_get = MagicMock(return_value=response) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + url = "https://tracker.example/download/book.torrent" + first = extract_torrent_info(url, fetch_torrent=True) + second = extract_torrent_info(url, fetch_torrent=True) + + mock_get.assert_called_once() + assert first.is_magnet is True + assert second.magnet_url == magnet + assert second.info_hash == "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0"