mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-10 12:28:27 +00:00
fix(requests): retrying a request after its client failed offers the same release again (#1677)
When a request's download fails in a way the queue can't resume, the request is reopened and its Retry button opens the release list. The release that just failed was drawn there as a red, disabled "Failed" button for as long as the failed task stayed in the queue status, so you couldn't pick it again even when the release was fine and the download client was the problem. A bad qBittorrent password is the easy way to hit it. Thanks @joshft91 for the report and the steps. What changes: - In the release list, a failed request download now shows Retry, the same as a failure the queue can retry already does. A direct download with nothing to retry from still shows Failed. The rule lives in one small helper used by the release modal and both button-state paths in `useDownloadTracking`. - A client that refuses the login is no longer held against the release. qbittorrent-api raises `LoginFailed` with no message for a wrong password, and only "Forbidden" or "Unauthorized" for a bad session or API key, so none of it matched the unreachable-client markers from #1655 and the release went into `failed_releases`. The qBittorrent client now says "qBittorrent authentication failed (...)" for these. - The request reopen and the auto-download retry used two copies of the same marker list. They now share one in `status_messages.py`, which also counts an explicit authentication failure as the client's fault. A bare 403 or 401 is not on it, since a dead .torrent link fails that way too and that one is the release's. Genuine release failures (no seeders, a rejected torrent, a dead .torrent link) are still remembered as #1655 intended. Tested with a scratch instance against a mock Prowlarr and a fake qBittorrent that rejects the login: before, the request's Retry opened the list with the failed release as a red X; after, it shows Retry, picking it fulfils the request again, and the request reopens without the release in `failed_releases`. New tests cover the qBittorrent messages, both reopen paths and the button state. Fixes #1674 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Failed releases are no longer skipped after download-client connection or login failures; download failures and stall cancellations remain eligible to be skipped on the next search. * Failed requests with an associated request can now show a retry option, while direct downloads without retries remain marked as failed. * qBittorrent login and authorization errors are reported more clearly. * **Documentation** * Clarified which failures cause a release to be skipped on a later search. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
efc0f3df6b
commit
76d7df306f
12 files changed
+375
-43
No files matched your search
@@ -43,7 +43,7 @@ A request whose book turns out to be in your library already is closed, not left
|
||||
|
||||
Hardcover allows a short burst of lookups and then answers 429. Each pass paces its book lookups and backs off when told to, following `Retry-After`. A lookup that still fails counts as an error for that request, not as "book not found", and the request is tried again next pass.
|
||||
|
||||
A download that fails, or is interrupted (a restart while it was queued, say), does not stay that way forever. Once the request has sat there for **Retry failed downloads after (days)** (7 by default), it goes back to pending and the next pass searches again. Each pass reopens at most five, oldest first, so a backlog is spread out instead of hitting your sources at once. An interrupted download may pick the same release again, since a torrent already in your client is joined instead of added twice. A download that itself failed, or that Shelfmark's stall timer cancelled, has its release remembered and skipped next time, and the request is given up on after three failed retries. Requests you or an admin cancelled are never retried, and neither are rejected ones: both are decisions. Set the value to 0 to turn retries off.
|
||||
A download that fails, or is interrupted (a restart while it was queued, say), does not stay that way forever. Once the request has sat there for **Retry failed downloads after (days)** (7 by default), it goes back to pending and the next pass searches again. Each pass reopens at most five, oldest first, so a backlog is spread out instead of hitting your sources at once. An interrupted download may pick the same release again, since a torrent already in your client is joined instead of added twice. A download that itself failed, or that Shelfmark's stall timer cancelled, has its release remembered and skipped next time (not when the download client could not be reached or refused Shelfmark's login: that says nothing about the release), and the request is given up on after three failed retries. Requests you or an admin cancelled are never retried, and neither are rejected ones: both are decisions. Set the value to 0 to turn retries off.
|
||||
|
||||
A request that no source has a match for is not searched every cycle for ever. After three passes in a row with no match it is searched once a day, then every two days, and no further apart than **Longest wait between searches for an unmatched request (days)** (3 by default, 0 searches every unmatched request every cycle). A match, a search that failed (which says nothing about the book) or **Sync now** starts it over.
|
||||
**Trying another release sooner.** Set **Try another release after a failed download (minutes)** (off by default) and a download that fails, or that is given up on (stalled, or no seeders when **Give Up With No Seeders** is on), does not wait for the days above. Between sync cycles Shelfmark looks for such requests, reopens them after that many minutes and searches again, for those requests only. The release that failed is skipped, and a request is still given up on after three failed attempts.
|
||||
|
||||
@@ -43,6 +43,7 @@ from shelfmark.core.request_policy import (
|
||||
resolve_policy_mode,
|
||||
)
|
||||
from shelfmark.core.request_validation import RequestStatus
|
||||
from shelfmark.core.status_messages import client_problem
|
||||
from shelfmark.core.text_match import (
|
||||
DEFAULT_TITLE_MATCH_THRESHOLD,
|
||||
PACKAGING_MARKERS,
|
||||
@@ -1030,15 +1031,6 @@ def _latest_history(db_path: str, request_ids: list[int]) -> dict[int, tuple[str
|
||||
return latest
|
||||
|
||||
|
||||
# What a download records when its client could not be reached. The release is not to blame.
|
||||
_CLIENT_UNREACHABLE = (
|
||||
"failed to connect",
|
||||
"cannot connect",
|
||||
"connection refused",
|
||||
"connection aborted",
|
||||
)
|
||||
|
||||
|
||||
def _failure_kind(latest: tuple[str, str | None] | None) -> str | None:
|
||||
"""Why a request whose delivery never completed never arrived, from its latest history row.
|
||||
|
||||
@@ -1054,8 +1046,9 @@ def _failure_kind(latest: tuple[str, str | None] | None) -> str | None:
|
||||
return _INTERRUPTED
|
||||
if final_status == "error":
|
||||
lowered = (message or "").strip().lower()
|
||||
if lowered == _INTERRUPTED_MESSAGE or any(m in lowered for m in _CLIENT_UNREACHABLE):
|
||||
# Nothing was wrong with the release: the process stopped, or its client was down.
|
||||
if lowered == _INTERRUPTED_MESSAGE or client_problem(lowered):
|
||||
# Nothing was wrong with the release: the process stopped, or its client was down
|
||||
# or refused the login.
|
||||
return _INTERRUPTED
|
||||
return _FAILED
|
||||
if final_status == "cancelled" and (message or "").strip().lower().startswith(
|
||||
|
||||
@@ -18,6 +18,7 @@ from shelfmark.core.request_validation import (
|
||||
normalize_policy_mode,
|
||||
validate_request_level_payload,
|
||||
)
|
||||
from shelfmark.core.status_messages import client_problem
|
||||
|
||||
MAX_REQUEST_NOTE_LENGTH = 1000
|
||||
MAX_REQUEST_JSON_BLOB_BYTES = 10 * 1024
|
||||
@@ -197,21 +198,6 @@ _DELIVERY_PRECEDENCE: dict[str, int] = {
|
||||
}
|
||||
|
||||
|
||||
# A download that failed because its client could not be reached says nothing about the release:
|
||||
# the same one is fine once the client is back, and it may already be downloading there.
|
||||
_CLIENT_UNREACHABLE_MARKERS = (
|
||||
"failed to connect",
|
||||
"cannot connect",
|
||||
"connection refused",
|
||||
"connection aborted",
|
||||
)
|
||||
|
||||
|
||||
def _client_unreachable(message: object) -> bool:
|
||||
text = str(message or "").lower()
|
||||
return any(marker in text for marker in _CLIENT_UNREACHABLE_MARKERS)
|
||||
|
||||
|
||||
def sync_delivery_states_from_queue_status(
|
||||
user_db: UserDB,
|
||||
*,
|
||||
@@ -291,7 +277,7 @@ def sync_delivery_states_from_queue_status(
|
||||
reopened = user_db.reopen_failed_request(
|
||||
request_id,
|
||||
failure_reason=failure_reason,
|
||||
remember_failed_release=not _client_unreachable(failure_reason),
|
||||
remember_failed_release=not client_problem(failure_reason),
|
||||
)
|
||||
if reopened is not None:
|
||||
updated.append(reopened)
|
||||
@@ -721,10 +707,11 @@ def reopen_failed_request(
|
||||
"""Reopen a failed fulfilled request so admins can re-approve with a new release.
|
||||
|
||||
The release that failed is noted on the request, so the next search does not pick it again,
|
||||
unless the failure was the client being unreachable, which says nothing about the release.
|
||||
unless the failure was the client being unreachable or refusing the login, which says nothing
|
||||
about the release.
|
||||
"""
|
||||
return user_db.reopen_failed_request(
|
||||
request_id,
|
||||
failure_reason=failure_reason,
|
||||
remember_failed_release=not _client_unreachable(failure_reason),
|
||||
remember_failed_release=not client_problem(failure_reason),
|
||||
)
|
||||
@@ -16,6 +16,25 @@ FAILURE_RETRY_MINUTES_SETTING = "AUTO_DOWNLOAD_FAILURE_RETRY_MINUTES"
|
||||
|
||||
_SHOWN_FOR = frozenset({"error", "cancelled"})
|
||||
|
||||
# What a download records when its client could not be reached or refused Shelfmark's login.
|
||||
# Bare "forbidden" or "unauthorized" is not on the list: a dead .torrent link fails that way too,
|
||||
# and that one is the release's fault. A client's login failure is reworded to say so.
|
||||
# Neither says anything about the release: the same one is fine once the client is back or its
|
||||
# settings are fixed, and it may already be downloading there.
|
||||
_CLIENT_PROBLEM_MARKERS = (
|
||||
"failed to connect",
|
||||
"cannot connect",
|
||||
"connection refused",
|
||||
"connection aborted",
|
||||
"authentication failed",
|
||||
)
|
||||
|
||||
|
||||
def client_problem(message: object) -> bool:
|
||||
"""Whether a failure message blames the download client rather than the release."""
|
||||
text = str(message or "").lower()
|
||||
return any(marker in text for marker in _CLIENT_PROBLEM_MARKERS)
|
||||
|
||||
|
||||
def _next_step(*, request_linked: bool, retry_minutes: int) -> str:
|
||||
if request_linked and retry_minutes > 0:
|
||||
|
||||
@@ -34,6 +34,7 @@ from shelfmark.download.network import get_ssl_verify
|
||||
try:
|
||||
import qbittorrentapi as _qbittorrentapi
|
||||
except ImportError:
|
||||
_qbittorrentapi = None
|
||||
_ImportedQBittorrentApiError = RuntimeError
|
||||
_ImportedQBittorrentLoginFailed = RuntimeError
|
||||
else:
|
||||
@@ -45,6 +46,7 @@ logger = setup_logger(__name__)
|
||||
_HASH_LENGTH_40 = 40
|
||||
_HASH_LENGTH_ED2K = 32
|
||||
_HTTP_STATUS_FORBIDDEN = HTTPStatus.FORBIDDEN
|
||||
_HTTP_STATUS_UNAUTHORIZED = HTTPStatus.UNAUTHORIZED
|
||||
_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
|
||||
@@ -103,6 +105,35 @@ def _resolve_qbittorrent_exception_type(candidate: object) -> type[Exception]:
|
||||
|
||||
_QBittorrentApiError = _resolve_qbittorrent_exception_type(_ImportedQBittorrentApiError)
|
||||
_QBittorrentLoginFailed = _resolve_qbittorrent_exception_type(_ImportedQBittorrentLoginFailed)
|
||||
|
||||
|
||||
def _qbittorrent_auth_errors(module: object) -> tuple[type[Exception], ...]:
|
||||
"""The qbittorrent-api errors that mean qBittorrent refused Shelfmark's login.
|
||||
|
||||
Empty when the library is missing, so nothing is mistaken for a refused login.
|
||||
"""
|
||||
found = (
|
||||
getattr(module, name, None) for name in ("LoginFailed", "HTTP401Error", "HTTP403Error")
|
||||
)
|
||||
return tuple(
|
||||
error for error in found if isinstance(error, type) and issubclass(error, Exception)
|
||||
)
|
||||
|
||||
|
||||
_QBITTORRENT_AUTH_ERRORS = _qbittorrent_auth_errors(_qbittorrentapi)
|
||||
|
||||
|
||||
def _auth_failure_message(error: Exception | None = None, *, status: int | None = None) -> str:
|
||||
"""Name a refused login as one, so the failure is not held against the release.
|
||||
|
||||
qbittorrent-api says nothing for a wrong password and only "Forbidden" or "Unauthorized"
|
||||
for a bad session or API key, which reads like a problem with the torrent.
|
||||
"""
|
||||
code = status if status is not None else getattr(error, "http_status_code", None)
|
||||
detail = f"HTTP {code}" if code else "login rejected"
|
||||
return f"qBittorrent authentication failed ({detail})"
|
||||
|
||||
|
||||
_QBITTORRENT_CLIENT_ERRORS = (
|
||||
_QBittorrentLoginFailed,
|
||||
_QBittorrentApiError,
|
||||
@@ -302,6 +333,10 @@ class QBittorrentClient(DownloadClient):
|
||||
return [SimpleNamespace(**t) for t in torrents], None
|
||||
except requests.exceptions.HTTPError as e:
|
||||
status = getattr(getattr(e, "response", None), "status_code", None)
|
||||
if status == _HTTP_STATUS_UNAUTHORIZED:
|
||||
msg = _auth_failure_message(status=status)
|
||||
logger.warning(msg)
|
||||
return [], msg
|
||||
if status:
|
||||
logger.warning("qBittorrent API error (HTTP %s): %s", status, e)
|
||||
return [], f"qBittorrent API request failed (HTTP {status})"
|
||||
@@ -321,6 +356,10 @@ class QBittorrentClient(DownloadClient):
|
||||
"qBittorrent URL is invalid (missing http:// or https://). "
|
||||
f"Configured: {self._base_url}",
|
||||
)
|
||||
except _QBITTORRENT_AUTH_ERRORS as e:
|
||||
msg = _auth_failure_message(e)
|
||||
logger.warning(msg)
|
||||
return [], msg
|
||||
except _QBITTORRENT_CLIENT_ERRORS as e:
|
||||
logger.debug("Failed to get torrents info: %s", e)
|
||||
return [], f"qBittorrent API error: {type(e).__name__}: {e}"
|
||||
@@ -608,6 +647,10 @@ class QBittorrentClient(DownloadClient):
|
||||
expected_hash,
|
||||
_METADATA_WAIT_POLLS * _METADATA_WAIT_INTERVAL_SECONDS,
|
||||
)
|
||||
except _QBITTORRENT_AUTH_ERRORS as e:
|
||||
msg = _auth_failure_message(e)
|
||||
logger.warning(msg)
|
||||
raise RuntimeError(msg) from e
|
||||
except _QBITTORRENT_CLIENT_ERRORS:
|
||||
logger.exception("qBittorrent add failed")
|
||||
raise
|
||||
|
||||
@@ -26,6 +26,7 @@ import type {
|
||||
import { isMetadataBook } from '../types';
|
||||
import { bookSupportsTargets } from '../utils/bookTargetLoader';
|
||||
import { getColorStyleFromHint } from '../utils/colorMaps';
|
||||
import { failedDownloadButtonState } from '../utils/failedDownloadButton';
|
||||
import { groupFilesByFormat, type FormatGroup } from '../utils/formatReview';
|
||||
import {
|
||||
LANGUAGE_OPTION_DEFAULT,
|
||||
@@ -1195,9 +1196,7 @@ const ReleaseModalSession = ({
|
||||
if (mode === 'blocked' || mode === 'request_book') {
|
||||
return { text: 'Unavailable', state: 'blocked' };
|
||||
}
|
||||
return currentStatus.error[releaseId].retry_available === true
|
||||
? { text: 'Retry', state: 'download' }
|
||||
: { text: 'Failed', state: 'error' };
|
||||
return failedDownloadButtonState(currentStatus.error[releaseId]);
|
||||
}
|
||||
// Check completed
|
||||
if (currentStatus.complete && currentStatus.complete[releaseId]) {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { useState, useCallback } from 'react';
|
||||
|
||||
import type { StatusData, ButtonStateInfo } from '../types';
|
||||
import { addFailedRelease, failedDownloadButtonState } from '../utils/failedDownloadButton';
|
||||
|
||||
interface UseDownloadTrackingReturn {
|
||||
bookToReleaseMap: Record<string, string[]>;
|
||||
@@ -12,6 +13,12 @@ interface UseDownloadTrackingReturn {
|
||||
getUniversalButtonState: (bookId: string) => ButtonStateInfo;
|
||||
}
|
||||
|
||||
/**
|
||||
* Download button state for books and releases, from the queue status and this session's picks.
|
||||
*
|
||||
* In universal mode a book's button follows the releases downloaded for it, so the hook keeps
|
||||
* which releases belong to which book and which books finished during this session.
|
||||
*/
|
||||
export function useDownloadTracking(currentStatus: StatusData): UseDownloadTrackingReturn {
|
||||
// Track mapping of metadata book IDs to release source IDs for universal mode
|
||||
const [bookToReleaseMap, setBookToReleaseMap] = useState<Record<string, string[]>>({});
|
||||
@@ -38,9 +45,7 @@ export function useDownloadTracking(currentStatus: StatusData): UseDownloadTrack
|
||||
const getButtonState = useCallback(
|
||||
(bookId: string): ButtonStateInfo => {
|
||||
if (currentStatus.error && currentStatus.error[bookId]) {
|
||||
return currentStatus.error[bookId].retry_available === true
|
||||
? { text: 'Retry', state: 'download' }
|
||||
: { text: 'Failed', state: 'error' };
|
||||
return failedDownloadButtonState(currentStatus.error[bookId]);
|
||||
}
|
||||
if (currentStatus.complete && currentStatus.complete[bookId]) {
|
||||
return { text: 'Downloaded', state: 'complete' };
|
||||
@@ -67,7 +72,11 @@ export function useDownloadTracking(currentStatus: StatusData): UseDownloadTrack
|
||||
[currentStatus],
|
||||
);
|
||||
|
||||
// Get button state for a metadata book in universal mode
|
||||
/**
|
||||
* The button for a metadata book in universal mode, from every release tracked for it. An
|
||||
* active download wins, then a finished one, then a failure that can be retried, then any
|
||||
* other failure.
|
||||
*/
|
||||
const getUniversalButtonState = useCallback(
|
||||
(bookId: string): ButtonStateInfo => {
|
||||
const releaseIds = bookToReleaseMap[bookId] || [];
|
||||
@@ -111,12 +120,7 @@ export function useDownloadTracking(currentStatus: StatusData): UseDownloadTrack
|
||||
bestState = { text: 'Downloaded', state: 'complete' };
|
||||
foundActiveState = true;
|
||||
} else if (currentStatus.error && currentStatus.error[releaseId]) {
|
||||
if (bestState.state === 'download') {
|
||||
bestState =
|
||||
currentStatus.error[releaseId].retry_available === true
|
||||
? { text: 'Retry', state: 'download' }
|
||||
: { text: 'Failed', state: 'error' };
|
||||
}
|
||||
bestState = addFailedRelease(bestState, currentStatus.error[releaseId]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
|
||||
import type { Book } from '../types';
|
||||
import { addFailedRelease, failedDownloadButtonState } from '../utils/failedDownloadButton';
|
||||
|
||||
const failed = (fields: Partial<Book>): Book => ({
|
||||
id: 'rel-1',
|
||||
title: 'Dungeon Crawler Carl',
|
||||
author: 'Matt Dinniman',
|
||||
...fields,
|
||||
});
|
||||
|
||||
describe('failedDownloadButtonState', () => {
|
||||
it('offers a failed request download again so the request can be retried with it', () => {
|
||||
expect(failedDownloadButtonState(failed({ request_id: 7, retry_available: false }))).toEqual({
|
||||
text: 'Retry',
|
||||
state: 'download',
|
||||
});
|
||||
});
|
||||
|
||||
it('offers a retry when the queue can retry the download', () => {
|
||||
expect(failedDownloadButtonState(failed({ retry_available: true }))).toEqual({
|
||||
text: 'Retry',
|
||||
state: 'download',
|
||||
});
|
||||
});
|
||||
|
||||
it('marks a direct download with nothing to retry from as failed', () => {
|
||||
expect(failedDownloadButtonState(failed({ retry_available: false }))).toEqual({
|
||||
text: 'Failed',
|
||||
state: 'error',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('addFailedRelease', () => {
|
||||
const get = { text: 'Get', state: 'download' } as const;
|
||||
const retryable = failed({ request_id: 7, retry_available: false });
|
||||
const final = failed({ id: 'rel-2', retry_available: false });
|
||||
|
||||
it('keeps Retry when a release that cannot be retried is found after it', () => {
|
||||
expect(addFailedRelease(addFailedRelease(get, retryable), final)).toEqual({
|
||||
text: 'Retry',
|
||||
state: 'download',
|
||||
});
|
||||
});
|
||||
|
||||
it('turns Failed into Retry when a retryable release is found after it', () => {
|
||||
expect(addFailedRelease(addFailedRelease(get, final), retryable)).toEqual({
|
||||
text: 'Retry',
|
||||
state: 'download',
|
||||
});
|
||||
});
|
||||
|
||||
it('marks the book failed when no failed release can be retried', () => {
|
||||
expect(addFailedRelease(get, final)).toEqual({ text: 'Failed', state: 'error' });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,32 @@
|
||||
import type { Book, ButtonStateInfo } from '../types';
|
||||
|
||||
const RETRY: ButtonStateInfo = { text: 'Retry', state: 'download' };
|
||||
const FAILED: ButtonStateInfo = { text: 'Failed', state: 'error' };
|
||||
|
||||
/**
|
||||
* The button for a release whose last download failed.
|
||||
*
|
||||
* When the queue cannot resume a failed download of a request, the request is reopened so a
|
||||
* release can be picked again, and the one that failed may be fine (its client was down or
|
||||
* refused the login). So it is offered again, like a download the queue can retry. Any other
|
||||
* failure with nothing to retry from stays marked as failed.
|
||||
*/
|
||||
export function failedDownloadButtonState(entry: Book): ButtonStateInfo {
|
||||
return entry.retry_available === true || (entry.request_id ?? 0) > 0
|
||||
? { ...RETRY }
|
||||
: { ...FAILED };
|
||||
}
|
||||
|
||||
/**
|
||||
* The button for a book with several tracked releases once another of them is found failed.
|
||||
*
|
||||
* A failure that can be retried wins over one that can't, in whichever order they are found, so
|
||||
* the book keeps Retry while any of its failed releases can be tried again.
|
||||
*/
|
||||
export function addFailedRelease(best: ButtonStateInfo, entry: Book): ButtonStateInfo {
|
||||
if (best.state === RETRY.state && best.text === RETRY.text) {
|
||||
return best;
|
||||
}
|
||||
const failed = failedDownloadButtonState(entry);
|
||||
return failed.text === RETRY.text || best.state === 'download' ? failed : best;
|
||||
}
|
||||
@@ -2621,6 +2621,9 @@ class TestRetryAfterCooldown:
|
||||
"Failed to add to qbittorrent: Failed to connect to qBittorrent. This i",
|
||||
"Cannot connect to qBittorrent at http://x",
|
||||
"Connection refused",
|
||||
"Failed to add to qbittorrent: qBittorrent authentication failed (HTTP 403)",
|
||||
"Failed to add to qbittorrent: qBittorrent authentication failed (HTTP 401)",
|
||||
"Failed to add to deluge: Deluge Web UI authentication failed",
|
||||
],
|
||||
)
|
||||
def test_a_download_whose_client_was_down_is_not_held_against_its_release(
|
||||
|
||||
@@ -835,6 +835,86 @@ def test_sync_delivery_states_does_not_blame_the_release_when_the_client_was_unr
|
||||
assert "failed_releases" not in refreshed["book_data"] # but the release is not held against
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"message",
|
||||
[
|
||||
"Failed to add to qbittorrent: qBittorrent authentication failed (HTTP 403)",
|
||||
"Failed to add to qbittorrent: qBittorrent authentication failed (HTTP 401)",
|
||||
"Failed to add to qbittorrent: qBittorrent authentication failed (login rejected)",
|
||||
"Failed to add to deluge: Deluge Web UI authentication failed",
|
||||
],
|
||||
)
|
||||
def test_sync_delivery_states_does_not_blame_the_release_when_the_client_refused_the_login(
|
||||
user_db, message
|
||||
):
|
||||
user = user_db.create_user(username="alice")
|
||||
fulfilled_request = user_db.create_request(
|
||||
user_id=user["id"],
|
||||
source_hint="prowlarr",
|
||||
content_type="audiobook",
|
||||
request_level="release",
|
||||
policy_mode="request_release",
|
||||
book_data=_book_data(),
|
||||
release_data={"source": "prowlarr", "source_id": "fine-rel", "title": "Fine"},
|
||||
status="fulfilled",
|
||||
delivery_state="queued",
|
||||
)
|
||||
|
||||
sync_delivery_states_from_queue_status(
|
||||
user_db,
|
||||
queue_status={
|
||||
"error": {
|
||||
"fine-rel": {
|
||||
"id": "fine-rel",
|
||||
"request_id": fulfilled_request["id"],
|
||||
"retry_available": False,
|
||||
"status_message": message,
|
||||
},
|
||||
},
|
||||
},
|
||||
user_id=user["id"],
|
||||
)
|
||||
|
||||
refreshed = user_db.get_request(fulfilled_request["id"])
|
||||
assert refreshed["status"] == "pending"
|
||||
assert "failed_releases" not in refreshed["book_data"]
|
||||
|
||||
|
||||
def test_sync_delivery_states_still_blames_a_release_whose_link_is_forbidden(user_db):
|
||||
"""A tracker's 403 on the .torrent link is the release's fault, so it is remembered."""
|
||||
user = user_db.create_user(username="alice")
|
||||
fulfilled_request = user_db.create_request(
|
||||
user_id=user["id"],
|
||||
source_hint="prowlarr",
|
||||
content_type="audiobook",
|
||||
request_level="release",
|
||||
policy_mode="request_release",
|
||||
book_data=_book_data(),
|
||||
release_data={"source": "prowlarr", "source_id": "dead-link", "title": "Dead"},
|
||||
status="fulfilled",
|
||||
delivery_state="queued",
|
||||
)
|
||||
|
||||
sync_delivery_states_from_queue_status(
|
||||
user_db,
|
||||
queue_status={
|
||||
"error": {
|
||||
"dead-link": {
|
||||
"id": "dead-link",
|
||||
"request_id": fulfilled_request["id"],
|
||||
"retry_available": False,
|
||||
"status_message": "403 Client Error: Forbidden for url: https://tracker.example/t/1",
|
||||
},
|
||||
},
|
||||
},
|
||||
user_id=user["id"],
|
||||
)
|
||||
|
||||
refreshed = user_db.get_request(fulfilled_request["id"])
|
||||
assert refreshed["status"] == "pending"
|
||||
assert [r.get("source_id") for r in refreshed["book_data"]["failed_releases"]] == ["dead-link"]
|
||||
|
||||
|
||||
def test_sync_delivery_states_preserves_retryable_error_state(user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
fulfilled_request = user_db.create_request(
|
||||
|
||||
@@ -1617,6 +1617,120 @@ class TestQBittorrentClientAddDownload:
|
||||
with pytest.raises(RuntimeError, match="Failed to add torrent: Fails\\."):
|
||||
client.add_download(magnet, "Test")
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("error_name", "error_text", "expected"),
|
||||
[
|
||||
("LoginFailed", "", "qBittorrent authentication failed (login rejected)"),
|
||||
("Forbidden403Error", "Forbidden", "qBittorrent authentication failed (HTTP 403)"),
|
||||
(
|
||||
"Unauthorized401Error",
|
||||
"Unauthorized",
|
||||
"qBittorrent authentication failed (HTTP 401)",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_add_download_names_a_rejected_login(
|
||||
self, monkeypatch, error_name, error_text, expected
|
||||
):
|
||||
"""A login qBittorrent refuses is reported as an authentication failure, not bare text."""
|
||||
import qbittorrentapi as real_qbittorrentapi
|
||||
|
||||
config_values = {
|
||||
"QBITTORRENT_URL": "http://localhost:8080",
|
||||
"QBITTORRENT_USERNAME": "admin",
|
||||
"QBITTORRENT_PASSWORD": "wrong",
|
||||
"QBITTORRENT_CATEGORY": "books",
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.clients.qbittorrent.config.get",
|
||||
lambda key, default="": config_values.get(key, default),
|
||||
)
|
||||
|
||||
valid_hash = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"
|
||||
mock_client_instance = MagicMock()
|
||||
error_type = getattr(real_qbittorrentapi, error_name)
|
||||
mock_client_instance.torrents_add.side_effect = error_type(error_text)
|
||||
fake_module = MagicMock(
|
||||
Client=MagicMock(return_value=mock_client_instance),
|
||||
APIError=real_qbittorrentapi.APIError,
|
||||
LoginFailed=real_qbittorrentapi.LoginFailed,
|
||||
HTTP401Error=real_qbittorrentapi.HTTP401Error,
|
||||
HTTP403Error=real_qbittorrentapi.HTTP403Error,
|
||||
)
|
||||
|
||||
with patch.dict("sys.modules", {"qbittorrentapi": fake_module}):
|
||||
import importlib
|
||||
|
||||
import shelfmark.download.clients.qbittorrent as qb_module
|
||||
|
||||
importlib.reload(qb_module)
|
||||
|
||||
client = qb_module.QBittorrentClient()
|
||||
magnet = f"magnet:?xt=urn:btih:{valid_hash}&dn=test"
|
||||
|
||||
with pytest.raises(RuntimeError) as raised:
|
||||
client.add_download(magnet, "Test")
|
||||
|
||||
assert str(raised.value) == expected
|
||||
|
||||
@staticmethod
|
||||
def _client_with_real_errors(monkeypatch, mock_client_instance):
|
||||
"""A QBittorrentClient over a mock session, with qbittorrent-api's real error types."""
|
||||
import importlib
|
||||
|
||||
import qbittorrentapi as real_qbittorrentapi
|
||||
|
||||
config_values = {
|
||||
"QBITTORRENT_URL": "http://localhost:8080",
|
||||
"QBITTORRENT_USERNAME": "admin",
|
||||
"QBITTORRENT_PASSWORD": "wrong",
|
||||
"QBITTORRENT_CATEGORY": "books",
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.clients.qbittorrent.config.get",
|
||||
lambda key, default="": config_values.get(key, default),
|
||||
)
|
||||
fake_module = MagicMock(
|
||||
Client=MagicMock(return_value=mock_client_instance),
|
||||
APIError=real_qbittorrentapi.APIError,
|
||||
LoginFailed=real_qbittorrentapi.LoginFailed,
|
||||
HTTP401Error=real_qbittorrentapi.HTTP401Error,
|
||||
HTTP403Error=real_qbittorrentapi.HTTP403Error,
|
||||
)
|
||||
with patch.dict("sys.modules", {"qbittorrentapi": fake_module}):
|
||||
import shelfmark.download.clients.qbittorrent as qb_module
|
||||
|
||||
importlib.reload(qb_module)
|
||||
return qb_module.QBittorrentClient()
|
||||
|
||||
def test_a_refused_login_while_checking_a_torrent_is_named_as_one(self, monkeypatch):
|
||||
"""A status check whose login is refused reports an authentication failure."""
|
||||
import qbittorrentapi as real_qbittorrentapi
|
||||
|
||||
mock_client_instance = MagicMock()
|
||||
mock_client_instance.auth_log_in.side_effect = real_qbittorrentapi.LoginFailed("")
|
||||
client = self._client_with_real_errors(monkeypatch, mock_client_instance)
|
||||
|
||||
torrents, error = client._request_torrent_info_records({"hashes": "abc"})
|
||||
|
||||
assert torrents == []
|
||||
assert error == "qBittorrent authentication failed (login rejected)"
|
||||
|
||||
def test_a_401_while_checking_a_torrent_is_an_authentication_failure(self, monkeypatch):
|
||||
"""HTTP 401 on the info call is the client's login, not the torrent."""
|
||||
import requests
|
||||
|
||||
response = MagicMock(status_code=401)
|
||||
response.raise_for_status.side_effect = requests.exceptions.HTTPError(response=response)
|
||||
mock_client_instance = MagicMock()
|
||||
mock_client_instance._session.get.return_value = response
|
||||
client = self._client_with_real_errors(monkeypatch, mock_client_instance)
|
||||
|
||||
torrents, error = client._request_torrent_info_records({"hashes": "abc"})
|
||||
|
||||
assert torrents == []
|
||||
assert error == "qBittorrent authentication failed (HTTP 401)"
|
||||
|
||||
def test_add_download_omits_empty_category(self, monkeypatch):
|
||||
"""Avoid sending empty category values to qBittorrent-compatible clients."""
|
||||
config_values = {
|
||||
|
||||
Reference in new issue
Block a user