Usenet album poll: tolerate SAB queue→history handoff, emit terminal failure (#706)
User reported usenet album downloads getting stuck on "downloading
release" while SABnzbd reported the job as complete. Container restart
did not help; reproducible on every usenet album download.
Three independent issues all causing the same symptom — the download
modal freezes mid-flow with no error surfaced to the user:
1. SAB queue → history transition window
SAB removes a slot from its queue BEFORE adding it to the history,
and on a busy server (par2 verify, unrar, multi-file move) that
window can span several poll iterations. The poll treated a single
None status as terminal failure ("disappeared from client") and
gave up. Now the poll tolerates up to ~10s of consecutive misses
(5 polls at the default 2s interval) before declaring the job gone.
2. SAB queue states like `Pp` were unmapped
`_SAB_QUEUE_STATE_MAP` didn't cover SAB's `Pp` (post-processing
summary), `Unpacking`, `Trying`, `Deleted`, or the `Prop_paused`
/ `Prop_failed` variants. Unmapped states fell through to the
default-'error' fallback, and the poll loop only treated explicit
'failed' / 'completed' as terminal — 'error' was neither, so the
loop spun until the 6-hour timeout. Map now covers every Status
value from SAB's `sabnzbd/api.py`, and the poll treats the default-
'error' fallback as a transient miss (warn-logged, retry within
the same tolerance window) so a brand-new unmapped state can't
infinite-loop the way `Pp` did here.
3. No terminal failure emit
The poll only logged on failure / timeout / disappeared — never
called the progress callback with 'failed', so the download modal
stayed at the last 'downloading' emit forever. Plumb a 'failed'
emit through every failure exit path so the UI flips out of the
downloading state when the poll gives up.
Plus:
4. SAB direct nzo_ids lookup instead of paging all-history
`_get_status_sync` was fetching the latest 50 history entries on
every poll and iterating to find the target nzo_id. On busy
servers (many recent downloads), the target job could roll past
the 50-entry window and look like a "disappeared" job. Replaced
with a targeted `mode=queue&nzo_ids=<id>` → `mode=history&nzo_ids=<id>`
chain. Falls back to the bulk path for SAB versions that pre-date
the nzo_ids filter — the transient-miss tolerance covers any
short-lived gap there too.
Implementation:
Lifted the album-bundle poll loop out of `usenet.py` and `torrent.py`
into `core/download_plugins/album_bundle.py:poll_album_download` —
near-duplicate implementations are now a single function with deps
injected so it's testable in isolation (kettui's extract-don't-AST-parse
standard; can't unit-test a `time.sleep` loop inside a plugin method).
The lifted helper takes:
- `get_status` callable bound to job_id, so the same loop works for
usenet UsenetStatus and torrent TorrentStatus shapes
- `complete_states` set so torrent's `{'seeding', 'completed'}` and
usenet's `{'completed'}` both Just Work
- `failed_states` set so torrent's `{'error'}` is terminal while
usenet's default-'error' fallback is transient
- `transient_miss_threshold` (default 5 ≈ 10s at 2s poll)
- `sleep` / `monotonic` injectables for deterministic tests
Per-track flows in both plugins gained the same transient-miss
tolerance inline — they don't use the emit pattern (update an
`active_downloads[id]` row dict via lock instead), so reusing the
helper would have required threading a no-op emit through. Inline
fix is small enough.
Tests:
- 11 new tests in `tests/test_album_bundle.py:poll_album_download`
cover the happy path, transient-miss tolerance with recovery,
hard-failure threshold, explicit-failed surface, timeout-emit,
default-'error' transient treatment, shutdown clean exit,
torrent's `seeding`-counts-as-complete, save_path captured across
iterations, and adapter-exception treated as transient miss.
- 521 download-suite tests pass (33 in test_album_bundle, others
pin existing torrent + usenet contracts).
- Ruff clean.
Closes #706.
This commit is contained in:
Broque Thomas
parent
1d6ced286b
commit
f13d339584
6 changed files with 561 additions and 83 deletions
|
|
@ -25,7 +25,7 @@ import shutil
|
|||
import time
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from typing import Iterable, Optional
|
||||
from typing import Any, Callable, Iterable, Optional
|
||||
|
||||
from config.settings import config_manager
|
||||
from utils.logging_config import get_logger
|
||||
|
|
@ -177,6 +177,139 @@ def atomic_copy_to_staging(src: Path, dest: Path) -> bool:
|
|||
raise
|
||||
|
||||
|
||||
# Number of consecutive None-status reads tolerated before treating the
|
||||
# job as gone. Sized for the SAB queue→history transition window: SAB
|
||||
# removes the slot from the queue before adding it to history, and on a
|
||||
# busy server (par2 verify + unrar) that window can be several poll
|
||||
# intervals. At the default 2s interval, 5 retries = ~10s of tolerance
|
||||
# before we give up and emit a terminal failure.
|
||||
DEFAULT_TRANSIENT_MISS_THRESHOLD = 5
|
||||
|
||||
|
||||
def poll_album_download(
|
||||
*,
|
||||
get_status: Callable[[], Optional[Any]],
|
||||
title: str,
|
||||
emit: Callable[..., None],
|
||||
complete_states: frozenset,
|
||||
failed_states: frozenset = frozenset(['failed']),
|
||||
is_shutdown: Optional[Callable[[], bool]] = None,
|
||||
transient_miss_threshold: int = DEFAULT_TRANSIENT_MISS_THRESHOLD,
|
||||
poll_interval: Optional[float] = None,
|
||||
timeout: Optional[float] = None,
|
||||
sleep: Callable[[float], None] = time.sleep,
|
||||
monotonic: Callable[[], float] = time.monotonic,
|
||||
log_prefix: str = '[album_bundle]',
|
||||
) -> Optional[str]:
|
||||
"""Drive the per-poll status loop for an album-bundle download.
|
||||
|
||||
Lifted out of ``UsenetDownloadPlugin._poll_album_download`` and the
|
||||
sibling torrent method so the loop is testable in isolation and so
|
||||
both plugins share the same exit semantics.
|
||||
|
||||
Contract:
|
||||
- ``get_status()`` returns the adapter status object for the bound
|
||||
job, ``None`` when the client doesn't know about the job
|
||||
currently (transient or terminal — disambiguated by retry count).
|
||||
- ``emit(state, **fields)`` is the plugin's progress callback —
|
||||
this function calls it on EVERY successful poll with
|
||||
``state='downloading'`` and ALWAYS calls it once more with
|
||||
``state='failed'`` before returning ``None`` on any failure path,
|
||||
so the UI doesn't freeze on the last 'downloading' emit.
|
||||
- ``complete_states`` is the adapter's terminal-success set
|
||||
('completed' alone for usenet; 'seeding' + 'completed' for
|
||||
torrent because seeding-but-files-on-disk also counts).
|
||||
- ``failed_states`` is the explicit-failure set. The adapter-level
|
||||
'error' (unmapped state default) is intentionally NOT in here —
|
||||
that's treated as a transient miss because a real SAB / NZBGet
|
||||
/ qBit never returns a literal 'error' state on a healthy job;
|
||||
it's only our default fallback for unknown queue strings. Real
|
||||
example: SAB's 'Pp' post-processing state was unmapped → became
|
||||
'error' → poll infinite-looped until the 6-hour timeout.
|
||||
- ``transient_miss_threshold`` is the number of consecutive None /
|
||||
'error' reads tolerated before declaring the job gone. Sized for
|
||||
the SAB queue→history gap window.
|
||||
|
||||
Returns the adapter's reported save_path on terminal success, or
|
||||
``None`` on any failure (timeout / disappeared / explicit failed
|
||||
/ shutdown). On every failure path emits ``'failed'`` once with an
|
||||
``error`` field describing why.
|
||||
"""
|
||||
interval = poll_interval if poll_interval is not None else get_poll_interval()
|
||||
deadline = monotonic() + (timeout if timeout is not None else get_poll_timeout())
|
||||
last_save_path: Optional[str] = None
|
||||
transient_misses = 0
|
||||
|
||||
def _fail(reason: str) -> None:
|
||||
try:
|
||||
emit('failed', release=title, error=reason)
|
||||
except Exception as cb_exc:
|
||||
logger.debug("%s terminal emit failed: %s", log_prefix, cb_exc)
|
||||
|
||||
while monotonic() < deadline:
|
||||
if is_shutdown and is_shutdown():
|
||||
# Shutdown is a clean exit — don't paint failure on the UI;
|
||||
# the app is going away anyway.
|
||||
return None
|
||||
|
||||
try:
|
||||
status = get_status()
|
||||
except Exception as e:
|
||||
logger.warning("%s Poll error: %s", log_prefix, e)
|
||||
status = None
|
||||
|
||||
if status is None:
|
||||
transient_misses += 1
|
||||
if transient_misses >= transient_miss_threshold:
|
||||
logger.error(
|
||||
"%s '%s' missing from client for %d consecutive polls — giving up",
|
||||
log_prefix, title, transient_misses,
|
||||
)
|
||||
_fail('Disappeared from client (no status after retries)')
|
||||
return None
|
||||
sleep(interval)
|
||||
continue
|
||||
|
||||
# Reset the miss counter only when the adapter returned a state
|
||||
# we actually recognise. The default-fallback 'error' is treated
|
||||
# as a continuing transient miss below, so it must NOT reset
|
||||
# here — otherwise a persistently-unmapped state loops forever.
|
||||
if status.state != 'error':
|
||||
transient_misses = 0
|
||||
|
||||
emit('downloading', progress=status.progress, downloaded=status.downloaded,
|
||||
speed=status.download_speed)
|
||||
if status.save_path:
|
||||
last_save_path = status.save_path
|
||||
|
||||
if status.state in complete_states:
|
||||
return last_save_path
|
||||
if status.state in failed_states:
|
||||
error = getattr(status, 'error', None) or 'Client reported failure'
|
||||
logger.error("%s '%s' failed: %s", log_prefix, title, error)
|
||||
_fail(error)
|
||||
return None
|
||||
if status.state == 'error':
|
||||
# Unmapped adapter state — see contract docstring. Warn so
|
||||
# we hear about new states the adapter map needs to grow
|
||||
# without breaking the user's download. The miss counter
|
||||
# was intentionally NOT reset above for this branch.
|
||||
logger.warning(
|
||||
"%s '%s' returned unmapped state — treating as transient",
|
||||
log_prefix, title,
|
||||
)
|
||||
transient_misses += 1
|
||||
if transient_misses >= transient_miss_threshold:
|
||||
_fail('Client returned unmapped state repeatedly')
|
||||
return None
|
||||
|
||||
sleep(interval)
|
||||
|
||||
logger.error("%s '%s' timed out", log_prefix, title)
|
||||
_fail('Download timed out')
|
||||
return None
|
||||
|
||||
|
||||
def copy_audio_files_atomically(
|
||||
sources: Iterable[Path], staging_dir: Path,
|
||||
) -> list:
|
||||
|
|
@ -206,11 +339,13 @@ __all__ = [
|
|||
"ALBUM_PICK_MAX_BYTES",
|
||||
"DEFAULT_POLL_INTERVAL_SECONDS",
|
||||
"DEFAULT_POLL_TIMEOUT_SECONDS",
|
||||
"DEFAULT_TRANSIENT_MISS_THRESHOLD",
|
||||
"atomic_copy_to_staging",
|
||||
"copy_audio_files_atomically",
|
||||
"get_poll_interval",
|
||||
"get_poll_timeout",
|
||||
"pick_best_album_release",
|
||||
"poll_album_download",
|
||||
"quality_score",
|
||||
"time",
|
||||
"unique_staging_path",
|
||||
|
|
|
|||
|
|
@ -59,10 +59,12 @@ from typing import Any, Dict, List, Optional, Tuple
|
|||
from config.settings import config_manager
|
||||
from core.archive_pipeline import collect_audio_after_extraction
|
||||
from core.download_plugins.album_bundle import (
|
||||
DEFAULT_TRANSIENT_MISS_THRESHOLD,
|
||||
copy_audio_files_atomically,
|
||||
get_poll_interval,
|
||||
get_poll_timeout,
|
||||
pick_best_album_release,
|
||||
poll_album_download,
|
||||
)
|
||||
from core.download_plugins.base import DownloadSourcePlugin
|
||||
from core.download_plugins.types import AlbumResult, DownloadStatus, TrackResult
|
||||
|
|
@ -297,6 +299,12 @@ class TorrentDownloadPlugin(DownloadSourcePlugin):
|
|||
|
||||
deadline = time.monotonic() + _POLL_TIMEOUT_SECONDS
|
||||
last_save_path: Optional[str] = None
|
||||
# Tolerate transient None reads — covers network blips. Torrent
|
||||
# adapters don't have an SAB-style queue→history transition,
|
||||
# but the same tolerance keeps a one-off connection failure
|
||||
# from killing an otherwise-healthy download.
|
||||
transient_misses = 0
|
||||
miss_threshold = DEFAULT_TRANSIENT_MISS_THRESHOLD
|
||||
while time.monotonic() < deadline:
|
||||
if self.shutdown_check and self.shutdown_check():
|
||||
return
|
||||
|
|
@ -307,9 +315,17 @@ class TorrentDownloadPlugin(DownloadSourcePlugin):
|
|||
status = None
|
||||
|
||||
if status is None:
|
||||
# Adapter forgot about the torrent — probably user-removed.
|
||||
self._mark_error(download_id, "Torrent disappeared from client")
|
||||
return
|
||||
transient_misses += 1
|
||||
if transient_misses >= miss_threshold:
|
||||
self._mark_error(
|
||||
download_id,
|
||||
f"Torrent disappeared from client (no status after {miss_threshold} polls)",
|
||||
)
|
||||
return
|
||||
time.sleep(_POLL_INTERVAL_SECONDS)
|
||||
continue
|
||||
|
||||
transient_misses = 0
|
||||
|
||||
with self._lock:
|
||||
row = self.active_downloads.get(download_id)
|
||||
|
|
@ -494,10 +510,32 @@ class TorrentDownloadPlugin(DownloadSourcePlugin):
|
|||
result['error'] = 'Torrent client refused the release'
|
||||
return result
|
||||
|
||||
# Phase 3: poll until complete.
|
||||
# Phase 3: poll until complete. The lifted helper handles
|
||||
# transient missing windows (uncommon for torrents — adapters
|
||||
# don't have a queue→history transition like SAB — but the
|
||||
# same path also catches network blips that would otherwise
|
||||
# take down the whole download) and always emits a terminal
|
||||
# 'failed' state on failure paths so the UI doesn't freeze on
|
||||
# the last 'downloading' emit.
|
||||
_emit('downloading', release=picked.title)
|
||||
save_path = self._poll_album_download(adapter, torrent_id, picked.title, _emit)
|
||||
save_path = poll_album_download(
|
||||
get_status=lambda: run_async(adapter.get_status(torrent_id)),
|
||||
title=picked.title,
|
||||
emit=_emit,
|
||||
# Torrent adapters flip to 'seeding' on completion (files
|
||||
# on disk, share-ratio progress) — both states count as
|
||||
# terminal success.
|
||||
complete_states=frozenset(['seeding', 'completed']),
|
||||
# qBit / Transmission / Deluge surface a real 'error'
|
||||
# state when the torrent itself errors (tracker, missing
|
||||
# files, etc.). That's distinct from the unmapped-state
|
||||
# default-'error' fallback the helper treats as transient.
|
||||
failed_states=frozenset(['error']),
|
||||
is_shutdown=self.shutdown_check,
|
||||
log_prefix='[Torrent album]',
|
||||
)
|
||||
if save_path is None:
|
||||
# poll_album_download already emitted terminal 'failed'.
|
||||
result['error'] = 'Torrent download failed or timed out'
|
||||
return result
|
||||
|
||||
|
|
@ -522,34 +560,6 @@ class TorrentDownloadPlugin(DownloadSourcePlugin):
|
|||
result['files'] = copied
|
||||
return result
|
||||
|
||||
def _poll_album_download(self, adapter, torrent_id, title, emit) -> Optional[str]:
|
||||
"""Poll the adapter until the torrent is complete. Returns
|
||||
the save path or ``None`` on timeout / failure."""
|
||||
deadline = time.monotonic() + _POLL_TIMEOUT_SECONDS
|
||||
last_save_path: Optional[str] = None
|
||||
while time.monotonic() < deadline:
|
||||
if self.shutdown_check and self.shutdown_check():
|
||||
return None
|
||||
try:
|
||||
status = run_async(adapter.get_status(torrent_id))
|
||||
except Exception as e:
|
||||
logger.warning("[Torrent album] Poll error: %s", e)
|
||||
status = None
|
||||
if status is None:
|
||||
logger.error("[Torrent album] '%s' disappeared from client", title)
|
||||
return None
|
||||
emit('downloading', progress=status.progress, downloaded=status.downloaded,
|
||||
speed=status.download_speed)
|
||||
if status.save_path:
|
||||
last_save_path = status.save_path
|
||||
if status.state in _COMPLETE_STATES:
|
||||
return last_save_path
|
||||
if status.state == 'error':
|
||||
logger.error("[Torrent album] '%s' errored: %s", title, status.error)
|
||||
return None
|
||||
time.sleep(_POLL_INTERVAL_SECONDS)
|
||||
logger.error("[Torrent album] '%s' timed out", title)
|
||||
return None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -22,8 +22,10 @@ from typing import Any, Dict, List, Optional, Tuple
|
|||
|
||||
from core.archive_pipeline import collect_audio_after_extraction
|
||||
from core.download_plugins.album_bundle import (
|
||||
DEFAULT_TRANSIENT_MISS_THRESHOLD,
|
||||
copy_audio_files_atomically,
|
||||
pick_best_album_release,
|
||||
poll_album_download,
|
||||
)
|
||||
from core.download_plugins.base import DownloadSourcePlugin
|
||||
from core.download_plugins.torrent import (
|
||||
|
|
@ -227,6 +229,14 @@ class UsenetDownloadPlugin(DownloadSourcePlugin):
|
|||
|
||||
deadline = time.monotonic() + _POLL_TIMEOUT_SECONDS
|
||||
last_save_path: Optional[str] = None
|
||||
# Tolerate transient None / 'error' (unmapped state) reads —
|
||||
# SAB removes a job from the queue before adding it to history,
|
||||
# and on a busy server that gap can span several polls. Same
|
||||
# bug class as the album-bundle path; see
|
||||
# ``core/download_plugins/album_bundle.py:poll_album_download``
|
||||
# docstring for the longer rationale.
|
||||
transient_misses = 0
|
||||
miss_threshold = DEFAULT_TRANSIENT_MISS_THRESHOLD
|
||||
while time.monotonic() < deadline:
|
||||
if self.shutdown_check and self.shutdown_check():
|
||||
return
|
||||
|
|
@ -237,8 +247,18 @@ class UsenetDownloadPlugin(DownloadSourcePlugin):
|
|||
status = None
|
||||
|
||||
if status is None:
|
||||
self._mark_error(download_id, "Usenet job disappeared from client")
|
||||
return
|
||||
transient_misses += 1
|
||||
if transient_misses >= miss_threshold:
|
||||
self._mark_error(
|
||||
download_id,
|
||||
f"Usenet job disappeared from client (no status after {miss_threshold} polls)",
|
||||
)
|
||||
return
|
||||
time.sleep(_POLL_INTERVAL_SECONDS)
|
||||
continue
|
||||
|
||||
if status.state != 'error':
|
||||
transient_misses = 0
|
||||
|
||||
with self._lock:
|
||||
row = self.active_downloads.get(download_id)
|
||||
|
|
@ -258,6 +278,18 @@ class UsenetDownloadPlugin(DownloadSourcePlugin):
|
|||
if status.state == 'failed':
|
||||
self._mark_error(download_id, status.error or "Usenet client reported failure")
|
||||
return
|
||||
if status.state == 'error':
|
||||
logger.warning(
|
||||
"Usenet poll: '%s' returned unmapped state — treating as transient",
|
||||
job_id,
|
||||
)
|
||||
transient_misses += 1
|
||||
if transient_misses >= miss_threshold:
|
||||
self._mark_error(
|
||||
download_id,
|
||||
"Usenet client returned unmapped state repeatedly",
|
||||
)
|
||||
return
|
||||
|
||||
time.sleep(_POLL_INTERVAL_SECONDS)
|
||||
|
||||
|
|
@ -408,8 +440,21 @@ class UsenetDownloadPlugin(DownloadSourcePlugin):
|
|||
return result
|
||||
|
||||
_emit('downloading', release=picked.title)
|
||||
save_path = self._poll_album_download(adapter, job_id, picked.title, _emit)
|
||||
save_path = poll_album_download(
|
||||
get_status=lambda: run_async(adapter.get_status(job_id)),
|
||||
title=picked.title,
|
||||
emit=_emit,
|
||||
# Usenet completes into history as 'completed'; no 'seeding'
|
||||
# equivalent. Failed is explicit on history failures.
|
||||
complete_states=frozenset(['completed']),
|
||||
failed_states=frozenset(['failed']),
|
||||
is_shutdown=self.shutdown_check,
|
||||
log_prefix='[Usenet album]',
|
||||
)
|
||||
if save_path is None:
|
||||
# poll_album_download already emitted the terminal 'failed'
|
||||
# state on every failure path (timeout / disappeared /
|
||||
# explicit failure / unmapped). UI is unstuck either way.
|
||||
result['error'] = 'Usenet download failed or timed out'
|
||||
return result
|
||||
|
||||
|
|
@ -433,29 +478,3 @@ class UsenetDownloadPlugin(DownloadSourcePlugin):
|
|||
result['files'] = copied
|
||||
return result
|
||||
|
||||
def _poll_album_download(self, adapter, job_id, title, emit) -> Optional[str]:
|
||||
deadline = time.monotonic() + _POLL_TIMEOUT_SECONDS
|
||||
last_save_path: Optional[str] = None
|
||||
while time.monotonic() < deadline:
|
||||
if self.shutdown_check and self.shutdown_check():
|
||||
return None
|
||||
try:
|
||||
status = run_async(adapter.get_status(job_id))
|
||||
except Exception as e:
|
||||
logger.warning("[Usenet album] Poll error: %s", e)
|
||||
status = None
|
||||
if status is None:
|
||||
logger.error("[Usenet album] '%s' disappeared from client", title)
|
||||
return None
|
||||
emit('downloading', progress=status.progress, downloaded=status.downloaded,
|
||||
speed=status.download_speed)
|
||||
if status.save_path:
|
||||
last_save_path = status.save_path
|
||||
if status.state in _COMPLETE_STATES:
|
||||
return last_save_path
|
||||
if status.state == 'failed':
|
||||
logger.error("[Usenet album] '%s' failed: %s", title, status.error)
|
||||
return None
|
||||
time.sleep(_POLL_INTERVAL_SECONDS)
|
||||
logger.error("[Usenet album] '%s' timed out", title)
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -21,26 +21,37 @@ from utils.logging_config import get_logger
|
|||
logger = get_logger("usenet.sabnzbd")
|
||||
|
||||
|
||||
# SAB queue states + history states → adapter-uniform set.
|
||||
# Queue: Idle, Paused, Downloading, Grabbing, Queued, Checking,
|
||||
# QuickCheck, Verifying, Repairing, Fetching, Extracting, Moving,
|
||||
# Running, Completed, Failed.
|
||||
# SAB queue states + history states → adapter-uniform set. Covers
|
||||
# every Status value from SAB's ``sabnzbd/api.py`` plus the legacy
|
||||
# short-form codes ("pp" for post-processing, "trying" for retry) and
|
||||
# the prop_* variants returned for items that bounce between paused
|
||||
# and failed during retry. Anything unmapped lands on "error" via
|
||||
# ``_map_state``'s default — the album-bundle poll helper treats that
|
||||
# default as a transient miss so a brand-new unmapped state can't
|
||||
# infinite-loop the poll the way "pp" used to.
|
||||
_SAB_QUEUE_STATE_MAP = {
|
||||
"idle": "queued",
|
||||
"queued": "queued",
|
||||
"grabbing": "queued",
|
||||
"fetching": "downloading",
|
||||
"downloading": "downloading",
|
||||
"paused": "paused",
|
||||
"checking": "verifying",
|
||||
"quickcheck": "verifying",
|
||||
"verifying": "verifying",
|
||||
"repairing": "repairing",
|
||||
"extracting": "extracting",
|
||||
"moving": "extracting",
|
||||
"running": "extracting",
|
||||
"completed": "completed",
|
||||
"failed": "failed",
|
||||
"idle": "queued",
|
||||
"queued": "queued",
|
||||
"grabbing": "queued",
|
||||
"fetching": "downloading",
|
||||
"downloading": "downloading",
|
||||
"trying": "downloading",
|
||||
"paused": "paused",
|
||||
"prop_paused": "paused",
|
||||
"checking": "verifying",
|
||||
"quickcheck": "verifying",
|
||||
"verifying": "verifying",
|
||||
"repairing": "repairing",
|
||||
"extracting": "extracting",
|
||||
"unpacking": "extracting",
|
||||
"moving": "extracting",
|
||||
"running": "extracting",
|
||||
"pp": "extracting",
|
||||
"postprocessing": "extracting",
|
||||
"completed": "completed",
|
||||
"failed": "failed",
|
||||
"prop_failed": "failed",
|
||||
"deleted": "failed",
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -156,7 +167,28 @@ class SABnzbdAdapter:
|
|||
return await loop.run_in_executor(None, self._get_status_sync, job_id)
|
||||
|
||||
def _get_status_sync(self, job_id: str) -> Optional[UsenetStatus]:
|
||||
# Check active queue first; if not found, fall back to history.
|
||||
# Direct nzo_ids lookup against queue, then history. Falls back
|
||||
# to the bulk fetch for SAB versions that don't honour the
|
||||
# nzo_ids filter (very old SAB), but the direct path is the hot
|
||||
# path because the bulk history fetch was limited to 50 entries
|
||||
# — on a busy SAB server a recently-completed job would roll
|
||||
# past the window and the poll would log "disappeared".
|
||||
if not job_id:
|
||||
return None
|
||||
queue = self._call_sync('queue', nzo_ids=job_id)
|
||||
if queue and isinstance(queue.get('queue'), dict):
|
||||
for slot in queue['queue'].get('slots', []) or []:
|
||||
if str(slot.get('nzo_id') or '') == job_id:
|
||||
return self._parse_queue_slot(slot)
|
||||
history = self._call_sync('history', nzo_ids=job_id)
|
||||
if history and isinstance(history.get('history'), dict):
|
||||
for slot in history['history'].get('slots', []) or []:
|
||||
if str(slot.get('nzo_id') or '') == job_id:
|
||||
return self._parse_history_slot(slot)
|
||||
# Fallback: SAB version pre-dating nzo_ids filter support. The
|
||||
# bulk path is still limit=50; the helper's transient-miss
|
||||
# tolerance will cover the gap if the entry briefly rolls out
|
||||
# of the window.
|
||||
for status in self._get_all_sync():
|
||||
if status.id == job_id:
|
||||
return status
|
||||
|
|
|
|||
|
|
@ -299,3 +299,284 @@ def test_get_poll_timeout_falls_back_on_garbage() -> None:
|
|||
assert get_poll_timeout() == DEFAULT_POLL_TIMEOUT_SECONDS
|
||||
cm.get.return_value = 0
|
||||
assert get_poll_timeout() == DEFAULT_POLL_TIMEOUT_SECONDS
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# poll_album_download — lifted poll loop for both torrent + usenet plugins.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
from core.download_plugins.album_bundle import poll_album_download
|
||||
|
||||
|
||||
@dataclass
|
||||
class _Status:
|
||||
"""Duck-typed sibling of UsenetStatus / TorrentStatus — only the
|
||||
fields poll_album_download reads."""
|
||||
state: str
|
||||
save_path: Optional[str] = None
|
||||
progress: float = 0.0
|
||||
downloaded: int = 0
|
||||
download_speed: int = 0
|
||||
error: Optional[str] = None
|
||||
|
||||
|
||||
class _ScriptedClock:
|
||||
"""Deterministic monotonic-time + sleep stand-in for poll tests.
|
||||
|
||||
Each call to ``sleep(n)`` advances ``now`` by ``n`` seconds with
|
||||
no real wall-clock delay. Lets us run multi-iteration poll
|
||||
scenarios in milliseconds and assert on the exact iteration count
|
||||
each branch took."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.now = 0.0
|
||||
self.sleep_calls = 0
|
||||
|
||||
def monotonic(self) -> float:
|
||||
return self.now
|
||||
|
||||
def sleep(self, seconds: float) -> None:
|
||||
self.now += seconds
|
||||
self.sleep_calls += 1
|
||||
|
||||
|
||||
def _make_emit_recorder():
|
||||
"""Collects (state, kwargs) tuples so tests can assert on the
|
||||
emit sequence the UI would see."""
|
||||
calls = []
|
||||
def _emit(state: str, **fields) -> None:
|
||||
calls.append((state, fields))
|
||||
return _emit, calls
|
||||
|
||||
|
||||
def test_poll_returns_save_path_on_completed_state() -> None:
|
||||
"""Happy path — adapter says 'completed' with a save_path on the
|
||||
first poll; function returns the path and emits a single
|
||||
'downloading' (NOT a terminal 'failed') so the caller can chain
|
||||
'staging' / 'staged' next."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
status = _Status(state='completed', save_path='/dl/album', progress=1.0)
|
||||
|
||||
result = poll_album_download(
|
||||
get_status=lambda: status,
|
||||
title='Album X',
|
||||
emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
|
||||
assert result == '/dl/album'
|
||||
states = [c[0] for c in calls]
|
||||
assert 'failed' not in states
|
||||
assert 'downloading' in states
|
||||
|
||||
|
||||
def test_poll_tolerates_transient_missing_during_sab_handoff() -> None:
|
||||
"""SAB removes a job from the queue before adding it to history.
|
||||
Pre-fix: one None read = give up + log 'disappeared from client'
|
||||
even though SAB was healthy and just mid-handoff. Now we tolerate
|
||||
up to ``transient_miss_threshold`` consecutive None reads before
|
||||
declaring the job gone. Recovery to a real status MUST reset the
|
||||
miss counter."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
sequence = iter([None, None, None,
|
||||
_Status(state='completed', save_path='/sab/done')])
|
||||
result = poll_album_download(
|
||||
get_status=lambda: next(sequence),
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
transient_miss_threshold=5,
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result == '/sab/done'
|
||||
assert 'failed' not in [c[0] for c in calls]
|
||||
|
||||
|
||||
def test_poll_gives_up_after_threshold_consecutive_misses() -> None:
|
||||
"""When the job genuinely is gone (user deleted it from SAB), the
|
||||
transient tolerance still has a floor — after N misses, fail
|
||||
explicitly and emit a terminal 'failed' so the UI doesn't freeze."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
result = poll_album_download(
|
||||
get_status=lambda: None,
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
transient_miss_threshold=3,
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=600.0,
|
||||
)
|
||||
assert result is None
|
||||
failed_calls = [c for c in calls if c[0] == 'failed']
|
||||
assert len(failed_calls) == 1
|
||||
assert 'Disappeared' in failed_calls[0][1].get('error', '')
|
||||
|
||||
|
||||
def test_poll_emits_terminal_failed_on_explicit_failed_state() -> None:
|
||||
"""Adapter says 'failed' (real failure, not transient). Function
|
||||
returns None AND emits 'failed' with the adapter's error message."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
status = _Status(state='failed', error='par2 unrecoverable')
|
||||
result = poll_album_download(
|
||||
get_status=lambda: status,
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result is None
|
||||
failed_calls = [c for c in calls if c[0] == 'failed']
|
||||
assert len(failed_calls) == 1
|
||||
assert failed_calls[0][1].get('error') == 'par2 unrecoverable'
|
||||
|
||||
|
||||
def test_poll_emits_terminal_failed_on_timeout() -> None:
|
||||
"""When the deadline passes without success or explicit failure,
|
||||
emit 'failed' once so the UI exits the 'downloading' state."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
status = _Status(state='downloading', progress=0.5)
|
||||
result = poll_album_download(
|
||||
get_status=lambda: status,
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=10.0,
|
||||
)
|
||||
assert result is None
|
||||
failed_calls = [c for c in calls if c[0] == 'failed']
|
||||
assert len(failed_calls) == 1
|
||||
assert 'timed out' in failed_calls[0][1].get('error', '').lower()
|
||||
|
||||
|
||||
def test_poll_treats_default_error_state_as_transient_not_terminal() -> None:
|
||||
"""The adapter state-map's default-fallback for unmapped strings
|
||||
is 'error' (real-world: SAB's 'Pp' state used to land here and
|
||||
cause the poll to infinite-loop because 'error' wasn't in the
|
||||
failed set and wasn't in the complete set). Now: treat as a
|
||||
transient miss so the poll recovers when the unmapped state
|
||||
transitions to a known one. If it stays unmapped for the threshold
|
||||
of consecutive polls, emit terminal failed."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
sequence = iter([
|
||||
_Status(state='error'),
|
||||
_Status(state='error'),
|
||||
_Status(state='completed', save_path='/done'),
|
||||
])
|
||||
result = poll_album_download(
|
||||
get_status=lambda: next(sequence),
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
transient_miss_threshold=5,
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result == '/done'
|
||||
assert 'failed' not in [c[0] for c in calls]
|
||||
|
||||
|
||||
def test_poll_gives_up_when_default_error_state_persists() -> None:
|
||||
"""If the adapter keeps returning the unmapped 'error' state past
|
||||
the threshold, fail rather than burning the full 6-hour timeout."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
result = poll_album_download(
|
||||
get_status=lambda: _Status(state='error'),
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
transient_miss_threshold=3,
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=600.0,
|
||||
)
|
||||
assert result is None
|
||||
failed_calls = [c for c in calls if c[0] == 'failed']
|
||||
assert len(failed_calls) == 1
|
||||
assert 'unmapped' in failed_calls[0][1].get('error', '').lower()
|
||||
|
||||
|
||||
def test_poll_shutdown_returns_none_without_terminal_emit() -> None:
|
||||
"""Process shutdown is a clean exit — don't paint failure on the
|
||||
UI; the app is going away anyway."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
result = poll_album_download(
|
||||
get_status=lambda: _Status(state='downloading', progress=0.5),
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
is_shutdown=lambda: True,
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result is None
|
||||
assert 'failed' not in [c[0] for c in calls]
|
||||
|
||||
|
||||
def test_poll_torrent_seeding_counts_as_complete() -> None:
|
||||
"""Torrent plugin passes ``complete_states={'seeding', 'completed'}``
|
||||
because qBit / Transmission flip the torrent to 'seeding' on
|
||||
completion (files already on disk + share ratio progress). Same
|
||||
poll function must accept either state as terminal success."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
status = _Status(state='seeding', save_path='/dl/album.torrent')
|
||||
result = poll_album_download(
|
||||
get_status=lambda: status,
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['seeding', 'completed']),
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result == '/dl/album.torrent'
|
||||
|
||||
|
||||
def test_poll_save_path_captured_across_iterations() -> None:
|
||||
"""save_path can appear mid-poll (e.g. once SAB moves the slot
|
||||
out of the queue and into history). The last non-empty save_path
|
||||
seen during the run is what we return on terminal success — even
|
||||
if the final status read happens to have it cleared."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
sequence = iter([
|
||||
_Status(state='downloading', progress=0.4),
|
||||
_Status(state='downloading', save_path='/late/path', progress=0.9),
|
||||
_Status(state='completed', progress=1.0),
|
||||
])
|
||||
result = poll_album_download(
|
||||
get_status=lambda: next(sequence),
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result == '/late/path'
|
||||
|
||||
|
||||
def test_poll_get_status_exception_treated_as_transient_miss() -> None:
|
||||
"""Adapter raising (network blip, JSON decode error) shouldn't
|
||||
blow up the poll thread — caught, logged, counted as a transient
|
||||
miss alongside None returns."""
|
||||
clock = _ScriptedClock()
|
||||
emit, calls = _make_emit_recorder()
|
||||
counter = {'n': 0}
|
||||
def _raising_then_success():
|
||||
counter['n'] += 1
|
||||
if counter['n'] <= 2:
|
||||
raise RuntimeError('network blip')
|
||||
return _Status(state='completed', save_path='/recovered')
|
||||
result = poll_album_download(
|
||||
get_status=_raising_then_success,
|
||||
title='Album X', emit=emit,
|
||||
complete_states=frozenset(['completed']),
|
||||
transient_miss_threshold=5,
|
||||
sleep=clock.sleep, monotonic=clock.monotonic,
|
||||
poll_interval=2.0, timeout=60.0,
|
||||
)
|
||||
assert result == '/recovered'
|
||||
assert 'failed' not in [c[0] for c in calls]
|
||||
|
|
|
|||
|
|
@ -3415,6 +3415,7 @@ function closeHelperSearch() {
|
|||
const WHATS_NEW = {
|
||||
'2.6.3': [
|
||||
{ unreleased: true },
|
||||
{ title: 'Fix: usenet album downloads stuck on "downloading release" even after SAB finished', desc: 'when SAB moves a job from its queue into history (post-processing handoff — verify + unrar window), there\'s a brief gap where the job is in neither. SoulSync\'s poll treated one missing read as terminal failure and gave up silently, leaving the download modal frozen on "downloading release" forever. SAB queue states like `Pp` (post-processing) also weren\'t in the state map, hit the default-error fallback, and hung the poll the same way. now the poll tolerates ~10 seconds of transient misses, treats unmapped queue states as transient instead of silently looping, fires a terminal failed event on every failure path so the modal exits the downloading state, and queries SAB by nzo_id directly instead of paging the last-50-entries history (avoids losing the job on busy servers). same tolerance applies to torrent downloads — covers network blips that used to kill an otherwise healthy download.' },
|
||||
{ title: 'Discogs: strip artist disambiguation suffixes everywhere', desc: 'Discogs marks duplicate-named artists with either `(N)` numeric suffixes ("Bullet (2)") or trailing asterisks ("John Smith*"). Only the `(N)` form was stripped, and only on the collection-import path — so artist + album search results in the UI surfaced raw "Foo*" or "Bar (2)" rows, and worse, import path inherited those characters into folder names on disk. centralized the cleanup into a single helper applied at every site a Discogs payload becomes a name string (artist search, album search, track lookups, get_artist_albums feature filtering). closes #634.' },
|
||||
{ title: 'Library: Enhanced / Standard view toggle now sticks per browser', desc: 'the artist detail page used to revert to Standard view every time you clicked into a new artist — flipping to Enhanced was a per-click thing with no memory. now the choice is persisted in localStorage and reapplied automatically on every artist open. survives page reloads too. non-admins (who can\'t toggle Enhanced anyway) are unaffected; admins who never touched the Enhanced view stay on Standard since the saved value defaults to it.' },
|
||||
{ title: 'Fix popup: manual matches survive Playlist Pipeline runs', desc: 'manually mapping a mirrored-playlist track via the Fix popup (search or MBID) saved correctly and the download went through, but the next Playlist Pipeline run kept reverting the match — the track flipped back to "Provider Changed" and you had to map it again. three things were ganging up on it: the manual-fix save always stamped `provider: \'spotify\'` even when the match came from MusicBrainz / iTunes / Deezer, so prepare-discovery saw a provider mismatch on the next run; the discovery worker also re-queued any matched_data that lacked `track_number` / `album.id` / `release_date` for re-discovery, and Fix-popup saves never carry those fields by design (search results don\'t include track_number, MBID lookups don\'t include album.id) — so manual fixes always looked "incomplete" and got overwritten; and the prepare-discovery provider check didn\'t honour the `manual_match` flag at all. all three patched — manual matches now stamp the actual source, the worker short-circuits on `manual_match` before the incomplete-data branch, and prepare-discovery treats manual fixes as cached regardless of provider drift.' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue