mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 20:51:10 +01:00
fix: Prowlarr expired download link refresh (#1109)
## Summary Fixes Prowlarr downloads that fail after a queued torrent result’s tracker download link expires. Prowlarr torrent results can expose a `downloadUrl` that is only a short-lived proxy to the upstream tracker. Some trackers, including MAM, include expiring credentials in that URL. Shelfmark was persisting that URL as retry data and later treating it as durable. If the in-memory Prowlarr cache was gone, or if qBittorrent tried to add a stale URL, Shelfmark could fail with a misleading torrent-hash error instead of refreshing the release. ## What changed - Stop persisting Prowlarr `downloadUrl` values as durable retry data. - Persist only source context needed to refresh the release later. - On a Prowlarr cache miss, re-query Prowlarr using the queued task context. - Accept refreshed results only when the stable identity matches the original release: - `guid == task.task_id` - or `infoUrl == task.task_id` - Cache the fresh raw Prowlarr result and build the download request from its current `downloadUrl` / `magnetUrl`. - If qBittorrent add fails with `Could not determine torrent hash from URL`, remove the stale cached Prowlarr result, refresh once, and retry with the fresh URL. - Preserve existing magnet handling: torrent results continue to use `magnetUrl` first, so the refresh path only targets Prowlarr proxy `downloadUrl` values that can expire. - Improve the user-facing failure when refresh cannot find the same release: `The indexer download link expired and the release could not be refreshed. Search again for a fresh result.` ## Why this approach The important constraint is avoiding accidental downloads of a different edition or format after the original tracker link expires. Re-running a search by title can return many plausible results, so the refresh path deliberately requires an exact stable identity match before using any new URL. This treats Prowlarr `downloadUrl` as a short-lived hint, while still allowing retries to recover when Prowlarr can find the same release again. The one-shot retry after qBittorrent add failure handles the case where Shelfmark still has a cached Prowlarr result, but that cached result contains an expired proxy URL. The refresh hook is source-specific and defaults to no-op for other external download handlers, so Newznab and other sources keep their existing retry behavior. ## Bug Fixes #1012 Context: https://github.com/calibrain/shelfmark/issues/1012#issuecomment-4917148398 ## Validation - `uv run pytest tests/prowlarr -q` - `uv run pytest tests/download/test_orchestrator_user_output_mode.py -q` - `uv run pytest tests/newznab/test_handler.py tests/audiobookbay/test_handler.py -q` - `uv run ruff check shelfmark/core/models.py shelfmark/download/orchestrator.py shelfmark/download/clients/base_handler.py shelfmark/release_sources/prowlarr/handler.py tests/prowlarr/test_handler.py tests/prowlarr/test_integration_handler.py tests/prowlarr/test_failure_scenarios.py tests/prowlarr/test_source.py tests/download/test_orchestrator_user_output_mode.py` - after resolving conflicts with latest `main`: `uv run pytest tests/prowlarr -q` - after resolving conflicts with latest `main`: `uv run ruff check shelfmark/release_sources/prowlarr/handler.py` - after resolving conflicts with latest `main`: `uv run pytest tests/prowlarr/test_handler.py tests/prowlarr/test_source.py -q` Co-authored-by: Aidan Abbott <aidanabbott@Aidans-MacBook-Pro.local>
This commit is contained in:
Aidan Abbott
co-authored by
Aidan Abbott
Aidan Abbott
parent
30d7f228be
commit
f8d3f990ca
@@ -205,7 +205,7 @@ def test_download_task_rejects_unavailable_source_before_handler(monkeypatch):
|
||||
orchestrator.get_handler.assert_not_called()
|
||||
|
||||
|
||||
def test_queue_release_persists_generic_retry_resolution_fields(monkeypatch):
|
||||
def test_queue_release_persists_prowlarr_retry_context_without_download_url(monkeypatch):
|
||||
import shelfmark.download.orchestrator as orchestrator
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
@@ -227,6 +227,7 @@ def test_queue_release_persists_generic_retry_resolution_fields(monkeypatch):
|
||||
"protocol": "torrent",
|
||||
"indexer": "MyIndexer",
|
||||
"extra": {
|
||||
"indexer_id": 12,
|
||||
"configured_ratio_limit": 1.25,
|
||||
"configured_seed_time_minutes": 90,
|
||||
"info_hash": "ABC123",
|
||||
@@ -239,14 +240,23 @@ def test_queue_release_persists_generic_retry_resolution_fields(monkeypatch):
|
||||
assert success is True
|
||||
assert error is None
|
||||
task = captured["task"]
|
||||
assert task.retry_download_url == "magnet:?xt=urn:btih:abc123"
|
||||
assert task.retry_download_protocol == "torrent"
|
||||
assert task.retry_download_url is None
|
||||
assert task.retry_download_protocol is None
|
||||
assert task.retry_source_context == {
|
||||
"source_id": "prowlarr-release-1",
|
||||
"indexer": "MyIndexer",
|
||||
"indexer_id": 12,
|
||||
}
|
||||
assert task.retry_release_name == "Queued Prowlarr Release"
|
||||
assert task.retry_expected_hash == "ABC123"
|
||||
assert task.retry_ratio_limit == 1.25
|
||||
assert task.retry_seeding_time_limit_minutes == 90
|
||||
assert task.can_retry_without_staged_source is True
|
||||
|
||||
payload = orchestrator.serialize_task_for_retry(task)
|
||||
assert payload["retry_download_url"] is None
|
||||
assert payload["retry_source_context"] == task.retry_source_context
|
||||
|
||||
|
||||
def test_queue_release_prefers_configured_seed_time_minutes_for_retry(monkeypatch):
|
||||
import shelfmark.download.orchestrator as orchestrator
|
||||
|
||||
@@ -520,7 +520,7 @@ class TestCacheFailures:
|
||||
|
||||
assert result is None
|
||||
assert recorder.had_error
|
||||
assert "not found in cache" in recorder.last_message.lower()
|
||||
assert "could not be refreshed" in recorder.last_message
|
||||
|
||||
def test_release_missing_download_url(self, handler, recorder, cancel_flag, sample_task):
|
||||
"""Handler should error when release has no download URL."""
|
||||
|
||||
@@ -15,6 +15,8 @@ from shelfmark.download.clients import (
|
||||
DownloadState,
|
||||
DownloadStatus,
|
||||
)
|
||||
from shelfmark.release_sources import Release, ReleaseProtocol
|
||||
from shelfmark.release_sources.prowlarr.cache import cache_release, remove_release
|
||||
from shelfmark.release_sources.prowlarr.handler import ProwlarrHandler
|
||||
from shelfmark.release_sources.prowlarr.utils import get_protocol
|
||||
|
||||
@@ -103,13 +105,16 @@ class TestProwlarrHandlerDownloadErrors:
|
||||
assert result is None
|
||||
assert recorder.last_status == "error"
|
||||
assert recorder.last_message is not None
|
||||
assert "cache" in recorder.last_message.lower()
|
||||
assert "could not be refreshed" in recorder.last_message
|
||||
|
||||
def test_resolve_download_uses_task_retry_fields_when_cache_is_missing(self):
|
||||
"""Generic retry fields should let restarts recover without the in-memory cache."""
|
||||
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=[],
|
||||
):
|
||||
handler = ProwlarrHandler()
|
||||
task = DownloadTask(
|
||||
@@ -122,14 +127,245 @@ class TestProwlarrHandlerDownloadErrors:
|
||||
retry_seeding_time_limit_minutes=60,
|
||||
retry_ratio_limit=1.5,
|
||||
)
|
||||
recorder = ProgressRecorder()
|
||||
|
||||
request = handler._resolve_download(task, lambda *_: None)
|
||||
request = handler._resolve_download(task, recorder.status_callback)
|
||||
|
||||
assert request is not None
|
||||
assert request.url == "magnet:?xt=urn:btih:abc123"
|
||||
assert request.protocol == "torrent"
|
||||
assert request.seeding_time_limit == 60
|
||||
assert request.ratio_limit == 1.5
|
||||
assert request is None
|
||||
assert recorder.last_status == "error"
|
||||
assert recorder.last_message is not None
|
||||
assert "could not be refreshed" in recorder.last_message
|
||||
|
||||
def test_cache_miss_re_resolves_and_uses_fresh_download_url(self):
|
||||
"""Cache miss should re-query Prowlarr and use a fresh exact-match URL."""
|
||||
task_id = "fresh-guid-1"
|
||||
fresh_url = "https://prowlarr.example.com/download/fresh-token"
|
||||
|
||||
def mock_search(*_args, **_kwargs):
|
||||
cache_release(
|
||||
task_id,
|
||||
{
|
||||
"guid": task_id,
|
||||
"protocol": "torrent",
|
||||
"downloadUrl": fresh_url,
|
||||
"title": "Fresh Release",
|
||||
},
|
||||
)
|
||||
return [
|
||||
Release(
|
||||
source="prowlarr",
|
||||
source_id=task_id,
|
||||
title="Fresh Release",
|
||||
info_url="https://tracker.example.com/release/1",
|
||||
protocol=ReleaseProtocol.TORRENT,
|
||||
)
|
||||
]
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.name = "qbittorrent"
|
||||
mock_client.find_existing.return_value = None
|
||||
mock_client.add_download.return_value = "download_id"
|
||||
|
||||
remove_release(task_id)
|
||||
try:
|
||||
with (
|
||||
patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search",
|
||||
side_effect=mock_search,
|
||||
) as mock_search_method,
|
||||
patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.get_client",
|
||||
return_value=mock_client,
|
||||
),
|
||||
patch.object(ProwlarrHandler, "_poll_and_complete", return_value=None),
|
||||
):
|
||||
handler = ProwlarrHandler()
|
||||
task = DownloadTask(
|
||||
task_id=task_id,
|
||||
source="prowlarr",
|
||||
title="Fresh Book",
|
||||
retry_source_context={"indexer": "MyIndexer"},
|
||||
)
|
||||
recorder = ProgressRecorder()
|
||||
|
||||
handler.download(
|
||||
task=task,
|
||||
cancel_flag=Event(),
|
||||
progress_callback=recorder.progress_callback,
|
||||
status_callback=recorder.status_callback,
|
||||
)
|
||||
|
||||
mock_search_method.assert_called_once()
|
||||
assert mock_client.add_download.call_args.kwargs["url"] == fresh_url
|
||||
finally:
|
||||
remove_release(task_id)
|
||||
|
||||
def test_stale_cached_url_add_failure_re_resolves_once_and_succeeds(self):
|
||||
"""Expired cached proxy URL should be refreshed once after qBittorrent hash failure."""
|
||||
task_id = "stale-guid-1"
|
||||
stale_url = "https://prowlarr.example.com/download/stale-token"
|
||||
fresh_url = "https://prowlarr.example.com/download/fresh-token"
|
||||
|
||||
def mock_search(*_args, **_kwargs):
|
||||
cache_release(
|
||||
task_id,
|
||||
{
|
||||
"guid": task_id,
|
||||
"protocol": "torrent",
|
||||
"downloadUrl": fresh_url,
|
||||
"title": "Fresh Release",
|
||||
},
|
||||
)
|
||||
return [
|
||||
Release(
|
||||
source="prowlarr",
|
||||
source_id=task_id,
|
||||
title="Fresh Release",
|
||||
protocol=ReleaseProtocol.TORRENT,
|
||||
)
|
||||
]
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.name = "qbittorrent"
|
||||
mock_client.find_existing.return_value = None
|
||||
mock_client.add_download.side_effect = [
|
||||
RuntimeError("Could not determine torrent hash from URL"),
|
||||
"download_id",
|
||||
]
|
||||
|
||||
cache_release(
|
||||
task_id,
|
||||
{
|
||||
"guid": task_id,
|
||||
"protocol": "torrent",
|
||||
"downloadUrl": stale_url,
|
||||
"title": "Stale Release",
|
||||
},
|
||||
)
|
||||
try:
|
||||
with (
|
||||
patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search",
|
||||
side_effect=mock_search,
|
||||
) as mock_search_method,
|
||||
patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.get_client",
|
||||
return_value=mock_client,
|
||||
),
|
||||
patch.object(ProwlarrHandler, "_poll_and_complete", return_value=None),
|
||||
):
|
||||
handler = ProwlarrHandler()
|
||||
task = DownloadTask(task_id=task_id, source="prowlarr", title="Stale Book")
|
||||
recorder = ProgressRecorder()
|
||||
|
||||
handler.download(
|
||||
task=task,
|
||||
cancel_flag=Event(),
|
||||
progress_callback=recorder.progress_callback,
|
||||
status_callback=recorder.status_callback,
|
||||
)
|
||||
|
||||
assert mock_client.add_download.call_count == 2
|
||||
assert mock_client.add_download.call_args_list[0].kwargs["url"] == stale_url
|
||||
assert mock_client.add_download.call_args_list[1].kwargs["url"] == fresh_url
|
||||
mock_search_method.assert_called_once()
|
||||
finally:
|
||||
remove_release(task_id)
|
||||
|
||||
def test_refresh_without_exact_match_fails_clearly(self):
|
||||
"""Refresh must not pick a different Prowlarr result when identity differs."""
|
||||
task_id = "missing-guid-1"
|
||||
other_id = "other-guid-1"
|
||||
|
||||
def mock_search(*_args, **_kwargs):
|
||||
cache_release(
|
||||
other_id,
|
||||
{
|
||||
"guid": other_id,
|
||||
"protocol": "torrent",
|
||||
"downloadUrl": "https://prowlarr.example.com/download/other",
|
||||
"title": "Other Release",
|
||||
},
|
||||
)
|
||||
return [
|
||||
Release(
|
||||
source="prowlarr",
|
||||
source_id=other_id,
|
||||
title="Other Release",
|
||||
protocol=ReleaseProtocol.TORRENT,
|
||||
)
|
||||
]
|
||||
|
||||
remove_release(task_id)
|
||||
remove_release(other_id)
|
||||
try:
|
||||
with patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search",
|
||||
side_effect=mock_search,
|
||||
):
|
||||
handler = ProwlarrHandler()
|
||||
task = DownloadTask(task_id=task_id, source="prowlarr", title="Missing Book")
|
||||
recorder = ProgressRecorder()
|
||||
|
||||
request = handler._resolve_download(task, recorder.status_callback)
|
||||
|
||||
assert request is None
|
||||
assert recorder.last_status == "error"
|
||||
assert recorder.last_message is not None
|
||||
assert "could not be refreshed" in recorder.last_message
|
||||
finally:
|
||||
remove_release(task_id)
|
||||
remove_release(other_id)
|
||||
|
||||
def test_magnet_result_does_not_trigger_prowlarr_url_refresh(self):
|
||||
"""Magnet failures should not be treated as expired Prowlarr proxy URLs."""
|
||||
task_id = "magnet-guid-1"
|
||||
magnet = "magnet:?xt=urn:btih:abc123&dn=test"
|
||||
mock_client = MagicMock()
|
||||
mock_client.name = "qbittorrent"
|
||||
mock_client.find_existing.return_value = None
|
||||
mock_client.add_download.side_effect = RuntimeError(
|
||||
"Could not determine torrent hash from URL"
|
||||
)
|
||||
|
||||
cache_release(
|
||||
task_id,
|
||||
{
|
||||
"guid": task_id,
|
||||
"protocol": "torrent",
|
||||
"downloadUrl": "https://prowlarr.example.com/download/stale-token",
|
||||
"magnetUrl": magnet,
|
||||
"title": "Magnet Release",
|
||||
},
|
||||
)
|
||||
try:
|
||||
with (
|
||||
patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search",
|
||||
return_value=[],
|
||||
) as mock_search_method,
|
||||
patch(
|
||||
"shelfmark.release_sources.prowlarr.handler.get_client",
|
||||
return_value=mock_client,
|
||||
),
|
||||
):
|
||||
handler = ProwlarrHandler()
|
||||
task = DownloadTask(task_id=task_id, source="prowlarr", title="Magnet Book")
|
||||
recorder = ProgressRecorder()
|
||||
|
||||
result = handler.download(
|
||||
task=task,
|
||||
cancel_flag=Event(),
|
||||
progress_callback=recorder.progress_callback,
|
||||
status_callback=recorder.status_callback,
|
||||
)
|
||||
|
||||
assert result is None
|
||||
assert mock_client.add_download.call_count == 1
|
||||
assert mock_client.add_download.call_args.kwargs["url"] == magnet
|
||||
mock_search_method.assert_not_called()
|
||||
finally:
|
||||
remove_release(task_id)
|
||||
|
||||
def test_download_fails_without_download_url(self):
|
||||
"""Test that download fails when release has no download URL."""
|
||||
|
||||
@@ -126,7 +126,7 @@ class TestHandlerCacheOperations:
|
||||
|
||||
assert result is None
|
||||
assert recorder.last_status == "error"
|
||||
assert "cache" in recorder.last_message.lower()
|
||||
assert "could not be refreshed" in recorder.last_message
|
||||
|
||||
def test_download_fails_without_download_url(self):
|
||||
"""Test that download fails when release has no download URL."""
|
||||
|
||||
@@ -519,8 +519,12 @@ class TestProwlarrLocalizedQueries:
|
||||
|
||||
assert success is True
|
||||
assert error is None
|
||||
assert captured_tasks[0].retry_download_url == secret_download_url
|
||||
assert captured_tasks[0].retry_download_protocol == "usenet"
|
||||
assert captured_tasks[0].retry_download_url is None
|
||||
assert captured_tasks[0].retry_download_protocol is None
|
||||
assert captured_tasks[0].retry_source_context == {
|
||||
"source_id": "secret-prowlarr-release",
|
||||
"info_url": "secret-prowlarr-release",
|
||||
}
|
||||
assert "retry_download_url" not in orchestrator._task_to_dict(captured_tasks[0])
|
||||
|
||||
def test_search_uses_localized_titles_when_available(self, monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user