Files
shelfmark/tests/core/test_releases_api_budget.py
T
CaliBrain 3d7ea40088 fix(search): reach the server's deadline, query one author (#1285, #1252) (#1287)
Two independent reasons a working search reported failure to the user.

1. The client gave up before the server did (#1285)

`/api/releases` bounds one release search with RELEASE_SEARCH_TIMEOUT
(default
300s) and answers a spent budget with a sentence naming the real cause -
the
machinery added for #1276. The frontend then aborted the direct_download
search
at a hard-coded 180s, so it always won the race: the user saw "Request
timed
out. Check your network connection or proxy configuration." instead, and
raising RELEASE_SEARCH_TIMEOUT changed nothing they could observe, the
180s
being baked into the hashed bundle inside the image.

- /api/config reports the effective (clamped) budget, and the client
derives its
  abort from it plus a margin, so the server always answers first.
- Direct-mode search shows what the server actually said. Every non-auth
failure
was relabelled "Unable to reach download source. Network may be
restricted or
mirrors blocked.", which discarded the explanation and blamed the user's
network. ApiResponseError now carries `serverMessage`, set only when the
server
  explained itself, so the status-line placeholder still falls back.

Two latency fixes for the cost that made the timeout reachable at all:

- Fetch each distinct AA search URL once per search. The language-filter
retry
re-runs every title variant, and with DIRECT_DOWNLOAD_LANGUAGE_FROM_PATH
on
both passes build a byte-identical URL - behind DDoS-Guard each repeat
is a
  fresh browser solve.
- Drop the solve-only bypass method. `_bypass_method_cdp_gui_click`
opens with
exactly that call and returns the moment it works, so the entry ahead of
it
could only repeat the half that had already failed, plus the backoff
before
the method that does work started. Reported at 0/19 successes and ~5.5s
of
  each ~26s solve against DDoS-Guard.

2. The query carried every contributor, not one author (#1252)

`_pick_search_author` returned `book.search_author` verbatim while the
authors[]
fallback beside it deliberately narrowed to the first name before a
comma. Both
fields routinely arrive holding every contributor joined with ", ": the
frontend
builds `book.author` as `authors.join(', ')` for display
(bookTransformers.ts)
and the release modal sends that display string straight back as the
`author`
parameter, and `browse_record_to_book_metadata` and the manual-search
branch
both split the joined text into `authors` while still passing the
unsplit string
as `search_author`, so the split was never used.

A book whose metadata lists translators was therefore searched for as

    Blindness Jose Saramago, Giovanni Pontiero, <persian translator>

which matches nothing on Anna's Archive. The bypass succeeds, the search
comes
back empty, and the user is told the book has no releases.

Narrowed in one place, `search_plan.first_author`, so the two branches
cannot
drift apart again, and applied to the IRC source, which built its query
with the
same verbatim preference. Hardcover is unaffected: it already sets
`search_author` from `_simplify_author_for_search(authors[0])`, which
resolves
"Last, First" itself and never yields a multi-author string.
2026-09-01 12:38:57 -04:00

181 lines
6.4 KiB
Python

"""GET /api/releases answers within a budget instead of outliving the caller.
Issue #1276: the endpoint is synchronous and had no deadline, while the bypass path it
reaches was allowed ~840s per URL. A protection challenge nobody could solve therefore
ran until the reverse proxy in front of Shelfmark gave up, and the user was shown
"Server unavailable (504). If using a reverse proxy, check its configuration." - which
names the wrong thing entirely.
"""
from __future__ import annotations
import importlib
from unittest.mock import patch
import pytest
from shelfmark.core import search_deadline
@pytest.fixture(scope="module")
def main_module():
with patch("shelfmark.download.orchestrator.start"):
import shelfmark.main as main
importlib.reload(main)
return main
@pytest.fixture
def client(main_module):
return main_module.app.test_client()
def _authenticate(client) -> None:
with client.session_transaction() as sess:
sess["user_id"] = "alice"
sess["is_admin"] = False
sess["db_user_id"] = 7
def _request(client, main_module, sources, search_impl):
"""Drive /api/releases with a stubbed source list and search implementation."""
class _Source:
def search(self, book, plan, *, expand_search=False, content_type="ebook"):
return search_impl(book, plan)
def get_column_config(self):
from shelfmark.release_sources import _default_column_config
return _default_column_config()
with (
patch.object(main_module, "get_auth_mode", return_value="none"),
patch("shelfmark.release_sources.list_available_sources", return_value=sources),
patch("shelfmark.release_sources.get_source", return_value=_Source()),
patch("shelfmark.release_sources.source_results_are_releases", return_value=False),
):
return client.get(
"/api/releases",
query_string={"provider": "manual", "book_id": "abc", "title": "Dune"},
)
def test_a_search_runs_under_a_budget(client, main_module):
"""The handler must put a deadline in force for whatever the sources do."""
_authenticate(client)
observed: list[float | None] = []
def _search(_book, _plan):
deadline = search_deadline.current()
observed.append(deadline.budget_seconds if deadline else None)
return []
resp = _request(client, main_module, [{"name": "direct_download", "enabled": True}], _search)
assert resp.status_code == 200
assert observed and observed[0] is not None, "no budget was in force during the search"
def test_the_budget_is_shared_across_sources(client, main_module):
"""A stuck first source must not spend the whole request on its own."""
_authenticate(client)
searched: list[str] = []
def _search(book, _plan):
searched.append(book.title)
# Whatever the first source did, it ran the clock out.
deadline = search_deadline.current()
if deadline is not None:
deadline.event.set()
return []
sources = [
{"name": "direct_download", "enabled": True},
{"name": "prowlarr", "enabled": True},
]
resp = _request(client, main_module, sources, _search)
assert len(searched) == 1, "the second source should not have been started"
assert "ran out of time" in resp.get_json()["error"]
def test_the_failure_carries_a_message_the_frontend_will_show(client, main_module):
"""The whole point of the budget.
Shelfmark answers 503 when a search comes back empty with errors, and the frontend
only substitutes its "Server unavailable ... check your reverse proxy" text when the
body carries no message of its own. So the budget has to produce a body that names
the protection challenge - and has to trip before the proxy's own timeout, where
there would be no body at all.
"""
_authenticate(client)
def _search(_book, _plan):
deadline = search_deadline.current()
if deadline is not None:
deadline.event.set()
return []
sources = [
{"name": "direct_download", "enabled": True},
{"name": "prowlarr", "enabled": True},
]
resp = _request(client, main_module, sources, _search)
message = resp.get_json()["error"]
assert "protection challenge" in message
assert "reverse proxy" not in message
# The source prefix is stripped by the handler; the sentence must survive intact.
assert message.startswith("The release search ran out of time")
def test_no_budget_leaks_out_of_the_request(client, main_module):
"""A queued download later on must not inherit a search's deadline."""
_authenticate(client)
_request(client, main_module, [{"name": "direct_download", "enabled": True}], lambda *_: [])
assert search_deadline.current() is None
def test_the_client_is_told_what_the_budget_is(client, main_module):
"""The browser has to outlast the server, or the message above never arrives.
The frontend puts its own AbortController on the direct_download search. That abort
was a fixed 180s while this budget defaults to 300s, so the client always gave up
first and replaced the sentence tested above with a generic network/proxy error -
and raising RELEASE_SEARCH_TIMEOUT changed nothing a user could see, because the
hard-coded 180s was in the hashed bundle inside the image. Reporting the budget lets
the client set its backstop behind it. See issue #1285.
"""
_authenticate(client)
with patch.object(main_module, "get_auth_mode", return_value="none"):
resp = client.get("/api/config")
assert resp.status_code == 200
reported = resp.get_json()["release_search_timeout"]
assert reported == search_deadline.budget_seconds()
assert reported > 0
def test_the_reported_budget_is_the_one_actually_enforced(client, main_module):
"""An out-of-range setting is clamped, so the raw config value would mislead."""
_authenticate(client)
with (
patch.object(main_module, "get_auth_mode", return_value="none"),
patch.object(
main_module.app_config,
"get",
side_effect=lambda key, default=None, **_kw: (
99999 if key == "RELEASE_SEARCH_TIMEOUT" else default
),
),
):
resp = client.get("/api/config")
reported = resp.get_json()["release_search_timeout"]
assert reported == search_deadline._MAX_SEARCH_BUDGET_SECONDS