mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-04 07:41:11 +01:00
fix: fetch tracker .torrent links once per add and surface fetch failures (#1115)
## 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
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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"<html>not a torrent</html>")
|
||||
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"
|
||||
|
||||
Reference in New Issue
Block a user