mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-06 06:34:38 +01:00
fix(qbittorrent): keep magnets whose metadata is still pending (#1282)
## Problem
`QBittorrentClient.add_download()` waits 20 × 0.5 s for qBittorrent to
leave `metaDL`, then raises:
```
Failed to add to qbittorrent: Torrent metadata resolution was not confirmed within the visibility grace period
(response=TorrentsAddedMetadata({'added_torrent_ids': [], 'failure_count': 0, 'pending_count': 1, 'success_count': 0}))
```
The wait exists to learn qBittorrent's primary torrent ID, which for
hybrid torrents switches from the v1 hash to the truncated v2 hash once
metadata resolves. A magnet on a thin public swarm routinely needs
longer than 10 s to find a peer that will serve metadata, and the
download is then abandoned even though the add itself succeeded. The
torrent stays in qBittorrent (`base_handler` logs "leaving in
qbittorrent") and often completes minutes later with nobody watching it.
Seen on v1.3.12 with public indexers through Prowlarr: every magnet-only
release failed this way, while `.torrent` releases from a private
indexer were fine. qBittorrent showed the same torrents at `metaDL 0%
seeds=0/0`, and they resolved on their own well after shelfmark had
given up.
## Change
Return the info hash we already have instead of raising when the grace
period expires. Reads then resolve either identity:
- `get_status()` and `get_download_path()` use `_resolve_torrent()`
instead of `_get_torrent_info()`, so a v1 hash still matches after
qBittorrent re-keys the torrent to v2. `_torrent_matches_download_id`
already compares `hash`, `infohash_v1` and `infohash_v2`.
- `remove()` and `set_category()` address the torrent by its current
primary hash through a new `_current_hash()` helper, which falls back to
the ID it was given when the torrent cannot be resolved.
- The two magic numbers become `_METADATA_WAIT_POLLS` and
`_METADATA_WAIT_INTERVAL_SECONDS`.
The happy path does not change. When metadata resolves inside the grace
period the resolved primary hash comes back as before, and
`_resolve_torrent()` tries the exact-hash lookup first, so it costs no
extra request.
## Tests
`test_add_fails_when_metadata_never_resolves` asserted the old
behaviour, so it becomes
`test_add_keeps_torrent_when_metadata_never_resolves` and asserts the
info hash is returned.
`test_get_status_resolves_hash_after_metadata_switch` is new: it reads
status by the v1 hash after qBittorrent reports the torrent under its v2
hash.
`uv run pytest tests/ --ignore=tests/e2e` gives the same 55 failures
with and without this change (they are all in `tests/bypass/` and need
Chrome, which my machine has no headless setup for), and
`tests/prowlarr/` is green at 524 passed. Ruff check and format are
clean. I have not run this branch against a live qBittorrent, so a
second pair of eyes on the `remove()` path would help.
This commit is contained in:
@@ -45,6 +45,10 @@ _HASH_LENGTH_ED2K = 32
|
||||
_HTTP_STATUS_FORBIDDEN = HTTPStatus.FORBIDDEN
|
||||
_HTTP_STATUS_NOT_FOUND = HTTPStatus.NOT_FOUND
|
||||
_METADATA_DOWNLOAD_STATES = {"forcedMetaDL", "metaDL"}
|
||||
# How long add_download waits for magnet metadata before falling back to the info
|
||||
# hash it already knows, rather than holding the download queue on a thin swarm.
|
||||
_METADATA_WAIT_POLLS = 20
|
||||
_METADATA_WAIT_INTERVAL_SECONDS = 0.5
|
||||
_ONE_WEEK_IN_SECONDS = 604800
|
||||
|
||||
|
||||
@@ -338,6 +342,24 @@ class QBittorrentClient(DownloadClient):
|
||||
None,
|
||||
)
|
||||
|
||||
def _current_hash(self, download_id: str) -> str:
|
||||
"""qBittorrent's current primary hash for any identity we know the torrent by.
|
||||
|
||||
Falls back to the given ID when the torrent cannot be found, so callers
|
||||
still address the hash they were handed and surface the client's error.
|
||||
"""
|
||||
try:
|
||||
torrent, error = self._resolve_torrent(download_id)
|
||||
except _QBITTORRENT_CLIENT_ERRORS as e:
|
||||
logger.debug("Could not resolve current hash for %s: %s", download_id, e)
|
||||
return download_id
|
||||
if error or not torrent:
|
||||
return download_id
|
||||
torrent_hash = getattr(torrent, "hash", None)
|
||||
if isinstance(torrent_hash, str) and torrent_hash:
|
||||
return torrent_hash
|
||||
return download_id
|
||||
|
||||
def _list_category_hashes(self, category: str | None) -> set[str] | None:
|
||||
"""Snapshot the hashes qBittorrent currently reports for a category."""
|
||||
torrents, error = self._list_torrents_by_category(category)
|
||||
@@ -495,9 +517,13 @@ class QBittorrentClient(DownloadClient):
|
||||
message = f"{message} (torrent file fetch failed: {torrent_info.fetch_error})"
|
||||
_raise_runtime_error(message)
|
||||
|
||||
# Wait until qBittorrent has resolved magnet metadata so the returned
|
||||
# hash is its stable primary torrent ID, which may differ from the v1 hash.
|
||||
for _ in range(20):
|
||||
# Prefer qBittorrent's primary torrent ID, which for hybrid torrents
|
||||
# switches from the v1 hash to the truncated v2 hash once metadata
|
||||
# resolves. A magnet with few peers can take minutes to fetch metadata,
|
||||
# and the torrent is worth keeping in the meantime: every lookup goes
|
||||
# through `_resolve_torrent`, which still matches the v1 hash against
|
||||
# `infohash_v1` after the primary ID has changed.
|
||||
for _ in range(_METADATA_WAIT_POLLS):
|
||||
torrent, error = self._resolve_torrent(expected_hash, category)
|
||||
if error:
|
||||
logger.debug("qBittorrent add_download: %s", error)
|
||||
@@ -506,12 +532,14 @@ class QBittorrentClient(DownloadClient):
|
||||
if isinstance(torrent_hash, str) and torrent_hash:
|
||||
logger.info("Added torrent: %s", torrent_hash)
|
||||
return torrent_hash.lower()
|
||||
time.sleep(0.5)
|
||||
time.sleep(_METADATA_WAIT_INTERVAL_SECONDS)
|
||||
|
||||
_raise_runtime_error(
|
||||
"Torrent metadata resolution was not confirmed within the visibility grace period "
|
||||
f"(response={result_text})"
|
||||
logger.info(
|
||||
"Added torrent %s; metadata still pending after %.0fs, tracking it by info hash",
|
||||
expected_hash,
|
||||
_METADATA_WAIT_POLLS * _METADATA_WAIT_INTERVAL_SECONDS,
|
||||
)
|
||||
return expected_hash.lower()
|
||||
except _QBITTORRENT_CLIENT_ERRORS:
|
||||
logger.exception("qBittorrent add failed")
|
||||
raise
|
||||
@@ -529,7 +557,7 @@ class QBittorrentClient(DownloadClient):
|
||||
|
||||
"""
|
||||
try:
|
||||
torrent, error = self._get_torrent_info(download_id)
|
||||
torrent, error = self._resolve_torrent(download_id)
|
||||
if error:
|
||||
return DownloadStatus.error(error)
|
||||
if not torrent:
|
||||
@@ -613,7 +641,8 @@ class QBittorrentClient(DownloadClient):
|
||||
|
||||
"""
|
||||
try:
|
||||
self._client.torrents_delete(torrent_hashes=download_id, delete_files=delete_files)
|
||||
torrent_hash = self._current_hash(download_id)
|
||||
self._client.torrents_delete(torrent_hashes=torrent_hash, delete_files=delete_files)
|
||||
logger.info(
|
||||
"Removed torrent from qBittorrent: %s%s",
|
||||
download_id,
|
||||
@@ -635,7 +664,7 @@ class QBittorrentClient(DownloadClient):
|
||||
logger.debug("Could not create category '%s': %s", category, e)
|
||||
|
||||
self._client.torrents_set_category(
|
||||
torrent_hashes=download_id,
|
||||
torrent_hashes=self._current_hash(download_id),
|
||||
category=category,
|
||||
)
|
||||
logger.info("Set qBittorrent category for %s to '%s'", download_id, category)
|
||||
@@ -657,7 +686,7 @@ class QBittorrentClient(DownloadClient):
|
||||
- join `save_path` with the torrent's top-level directory
|
||||
"""
|
||||
try:
|
||||
torrent, error = self._get_torrent_info(download_id)
|
||||
torrent, error = self._resolve_torrent(download_id)
|
||||
if error:
|
||||
logger.debug("qBittorrent get_download_path: %s", error)
|
||||
return None
|
||||
|
||||
@@ -919,8 +919,8 @@ class TestQBittorrentClientAddDownload:
|
||||
{"category": "audiobooks"},
|
||||
]
|
||||
|
||||
def test_add_fails_when_metadata_never_resolves(self, monkeypatch):
|
||||
"""Fail rather than return a transitional hash after the metadata timeout."""
|
||||
def test_add_keeps_torrent_when_metadata_never_resolves(self, monkeypatch):
|
||||
"""Return the info hash rather than abandon a magnet whose metadata is slow."""
|
||||
config_values = {
|
||||
"QBITTORRENT_URL": "http://localhost:8080",
|
||||
"QBITTORRENT_USERNAME": "admin",
|
||||
@@ -957,8 +957,49 @@ class TestQBittorrentClientAddDownload:
|
||||
|
||||
client = qb_module.QBittorrentClient()
|
||||
magnet = f"magnet:?xt=urn:btih:{v1_hash}&dn=test"
|
||||
with pytest.raises(RuntimeError, match="metadata resolution was not confirmed"):
|
||||
client.add_download(magnet, "Test Download")
|
||||
|
||||
assert client.add_download(magnet, "Test Download") == v1_hash
|
||||
|
||||
def test_get_status_resolves_hash_after_metadata_switch(self, monkeypatch):
|
||||
"""Track a torrent by its v1 hash after qBittorrent re-keys it to v2."""
|
||||
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),
|
||||
)
|
||||
|
||||
v1_hash = "edf46c7f938a3c678081734d7bff8b9c652ba5e5"
|
||||
v2_hash = "0bed5f40753b342cb143e83c2b21924cc8474731"
|
||||
full_v2_hash = "0bed5f40753b342cb143e83c2b21924cc847473134e44d1bd300bdc58c13010f"
|
||||
resolved_torrent = MockTorrent(
|
||||
hash_val=v2_hash,
|
||||
state="downloading",
|
||||
infohash_v1=v1_hash,
|
||||
infohash_v2=full_v2_hash,
|
||||
)
|
||||
mock_client_instance = MagicMock()
|
||||
mock_client_instance._session.get.side_effect = [
|
||||
create_mock_session_response([]),
|
||||
create_mock_session_response([resolved_torrent]),
|
||||
]
|
||||
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)
|
||||
|
||||
client = qb_module.QBittorrentClient()
|
||||
status = client.get_status(v1_hash)
|
||||
|
||||
assert status.state.value == "downloading"
|
||||
|
||||
def test_add_download_uses_expected_hash_without_fetch(self, monkeypatch):
|
||||
"""Skip proxy fetch when expected hash is provided for URL torrents."""
|
||||
|
||||
Reference in New Issue
Block a user