mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 18:01:04 +01:00
Fix silent Hardcover search failures on rejected sort values (#1183)
Hardcover forwards the `sort` argument to Typesense's `sort_by` and
rejects
the entire search if it dislikes the value -- an unknown field, a bare
field
name with no direction, or more than three sort keys. A rejected search
is
not a GraphQL error: it comes back as HTTP 200, no `errors` key, and a
null
`results` body.
_extract_typesense_hits() reads that null as `hits=[], found=0`, so a
failed
search was indistinguishable from one that matched nothing. Users saw
zero
results with a healthy container and no log line explaining why.
Add _execute_search_query(), used by the three sort-bearing call sites
(book
search, field typeahead, series resolution):
- Detect the rejection via the null `results` body. A search that
genuinely
matched nothing still returns a results object with `found: 0`, so empty
result sets are not mistaken for failures.
- Retry once with an empty sort, which Hardcover always accepts, so
searches
return results instead of nothing.
- Keep that fallback sticky for 15 minutes so every subsequent search
does
not pay for a request known to fail, and let it expire so sort order
comes
back on its own if the index is fixed upstream.
- Log rejections that no sort can explain, and retries that also fail,
at
ERROR instead of discarding them.
While the fallback is active, results fall back to Typesense's default
ordering regardless of the selected sort. Degraded ordering beats no
results,
and it is now logged rather than silent.
SORT_MAPPING itself is unchanged: all five of its values were verified
against
the live API and return results. The `sort: "relevance"` reported in
#1179 was
the raw SortOrder value sent by v1.3.5; the mapping already fixed that.
What
remained unfixed, and is fixed here, is that the failure was invisible.
Fixes #1179
This commit is contained in:
@@ -59,6 +59,10 @@ ENV FLASK_PORT=8084
|
||||
# Configure locale, timezone, and perform initial cleanup in a single layer
|
||||
RUN apt-get update && \
|
||||
apt-get install -y --no-install-recommends \
|
||||
# For building C-extensions (cffi, gevent, etc.)
|
||||
gcc \
|
||||
libffi-dev \
|
||||
python3-dev \
|
||||
# For locale
|
||||
locales tzdata \
|
||||
# For healthcheck
|
||||
|
||||
@@ -44,6 +44,7 @@ Works great alongside the following library tools, with support for automatic im
|
||||
### Prerequisites
|
||||
|
||||
- Docker & Docker Compose
|
||||
- At least 2 GB of RAM available to the container when using the standard image — see [Memory Requirements](#memory-requirements)
|
||||
|
||||
### Installation
|
||||
|
||||
@@ -138,6 +139,17 @@ docker compose up -d
|
||||
|
||||
The full-featured image with all network capabilities included.
|
||||
|
||||
#### Memory Requirements
|
||||
|
||||
The standard image ships a real Chromium browser, which it launches to solve Cloudflare challenges for Direct Download. Chromium needs room to run:
|
||||
|
||||
- **2 GB of RAM available to the container** is a safe minimum; 1 GB or less is where problems usually start
|
||||
- Only relevant if you use Direct Download. Prowlarr, IRC and audiobook sources don't start the browser
|
||||
|
||||
When the container is starved of memory, Chromium fails to start and every Direct Download fails with unrelated-looking errors — repeated `403 detected; switching to bypasser` followed by `No download URL found`, and downloads that never complete. If you're seeing that, check the container's memory limit and the host's free memory before suspecting your ISP or DNS.
|
||||
|
||||
If you can't spare the memory, use the [Lite](#lite) image with an external resolver (e.g. FlareSolverr) running elsewhere.
|
||||
|
||||
#### Tor Routing
|
||||
Optional Tor support for network privacy:
|
||||
```bash
|
||||
@@ -175,6 +187,7 @@ A lighter image without the built-in browser automation. Ideal for:
|
||||
- **External services** - Already running FlareSolverr or similar for other applications
|
||||
- **Alternative sources** - Using Prowlarr, IRC, or other configured sources
|
||||
- **Audiobooks** - Using Shelfmark primarily for audiobooks
|
||||
- **Constrained hosts** - No bundled browser, so it runs comfortably below the standard image's [memory requirements](#memory-requirements)
|
||||
|
||||
```bash
|
||||
curl -O https://raw.githubusercontent.com/calibrain/shelfmark/main/compose/docker-compose.lite.yml
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Hardcover.app metadata provider. Requires API key."""
|
||||
|
||||
import re
|
||||
import time
|
||||
from contextlib import suppress
|
||||
from dataclasses import dataclass
|
||||
from datetime import UTC, datetime
|
||||
@@ -553,6 +554,31 @@ TITLE_SUGGESTION_FIELDS = "title,alternative_titles"
|
||||
TITLE_SUGGESTION_WEIGHTS = "5,2"
|
||||
TITLE_SUGGESTION_SORT = "_text_match:desc,users_count:desc"
|
||||
|
||||
# Hardcover forwards `sort` to Typesense's `sort_by` and rejects the whole search
|
||||
# if it does not like the value -- an unknown field, a bare field name with no
|
||||
# direction, more than three keys. A rejected search comes back as HTTP 200 with
|
||||
# no GraphQL errors and a null `results` body, which is otherwise indistinguishable
|
||||
# from "nothing matched". An empty sort is always accepted, so fall back to it and
|
||||
# keep the fallback sticky for a while rather than paying for a doomed request on
|
||||
# every search.
|
||||
SORT_FALLBACK = ""
|
||||
SORT_FALLBACK_TTL = 900.0
|
||||
_sort_fallback_until = 0.0
|
||||
|
||||
|
||||
def _search_payload_rejected(result: dict[str, Any] | None) -> bool:
|
||||
"""Report whether Hardcover answered a search with a null results body.
|
||||
|
||||
A search that genuinely matched nothing still returns a results object with
|
||||
``found: 0``; only a rejected search nulls it out entirely.
|
||||
"""
|
||||
if not isinstance(result, dict):
|
||||
return False
|
||||
root = result.get("search", result)
|
||||
if not isinstance(root, dict) or "results" not in root:
|
||||
return False
|
||||
return root["results"] is None
|
||||
|
||||
|
||||
def _combine_headline_description(headline: str | None, description: str | None) -> str | None:
|
||||
"""Combine headline (tagline) and description into a single description."""
|
||||
@@ -1200,7 +1226,7 @@ class HardcoverProvider(MetadataProvider):
|
||||
if not self.api_key or len(normalized_query) < HARDCOVER_MIN_TYPEAHEAD_QUERY_LENGTH:
|
||||
return []
|
||||
|
||||
result = self._execute_query(
|
||||
result = self._execute_search_query(
|
||||
SEARCH_FIELD_OPTIONS_QUERY,
|
||||
{
|
||||
"query": normalized_query,
|
||||
@@ -1431,7 +1457,7 @@ class HardcoverProvider(MetadataProvider):
|
||||
logger.debug("Invalid Hardcover series id field value: %s", normalized_value)
|
||||
return None
|
||||
|
||||
result = self._execute_query(
|
||||
result = self._execute_search_query(
|
||||
SEARCH_FIELD_OPTIONS_QUERY,
|
||||
{
|
||||
"query": normalized_value,
|
||||
@@ -2386,7 +2412,7 @@ class HardcoverProvider(MetadataProvider):
|
||||
variables["weights"] = search_weights
|
||||
|
||||
try:
|
||||
result = self._execute_query(graphql_query, variables)
|
||||
result = self._execute_search_query(graphql_query, variables)
|
||||
if not result:
|
||||
logger.debug("Hardcover search: No result from API")
|
||||
return SearchResult(books=[], page=options.page, total_found=0, has_more=False)
|
||||
@@ -2654,6 +2680,45 @@ class HardcoverProvider(MetadataProvider):
|
||||
raise RuntimeError(msg) from e
|
||||
return None
|
||||
|
||||
def _execute_search_query(self, query: str, variables: dict[str, Any]) -> dict | None:
|
||||
"""Execute a search query, retrying without ``sort`` if Hardcover rejects it.
|
||||
|
||||
Returns None when the search was rejected, so callers report an empty
|
||||
result rather than silently treating a failure as "nothing matched".
|
||||
"""
|
||||
global _sort_fallback_until
|
||||
|
||||
sort = variables.get("sort")
|
||||
if sort and time.monotonic() < _sort_fallback_until:
|
||||
variables = {**variables, "sort": SORT_FALLBACK}
|
||||
sort = None
|
||||
|
||||
result = self._execute_query(query, variables)
|
||||
if not _search_payload_rejected(result):
|
||||
return result
|
||||
|
||||
if not sort:
|
||||
logger.error(
|
||||
"Hardcover rejected this search (query_type=%s, fields=%s) and returned "
|
||||
"no result body",
|
||||
variables.get("queryType", "Book"),
|
||||
variables.get("fields"),
|
||||
)
|
||||
return None
|
||||
|
||||
logger.warning(
|
||||
"Hardcover rejected sort '%s'; retrying searches without a sort order for %ss",
|
||||
sort,
|
||||
int(SORT_FALLBACK_TTL),
|
||||
)
|
||||
_sort_fallback_until = time.monotonic() + SORT_FALLBACK_TTL
|
||||
|
||||
retry = self._execute_query(query, {**variables, "sort": SORT_FALLBACK})
|
||||
if _search_payload_rejected(retry):
|
||||
logger.error("Hardcover rejected this search even without a sort order")
|
||||
return None
|
||||
return retry
|
||||
|
||||
def _parse_search_result(self, item: dict) -> BookMetadata | None:
|
||||
"""Parse a search result item into BookMetadata."""
|
||||
try:
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from shelfmark.metadata_providers import MetadataSearchOptions
|
||||
from shelfmark.metadata_providers.hardcover import HardcoverProvider
|
||||
|
||||
# Hardcover answers a rejected search with HTTP 200, no GraphQL errors, and a
|
||||
# null results body. A search that genuinely matched nothing still returns a
|
||||
# results object with found: 0.
|
||||
REJECTED = {"search": {"results": None}}
|
||||
EMPTY = {"search": {"results": {"hits": [], "found": 0}}}
|
||||
ONE_HIT = {"search": {"results": {"hits": [{"document": {"id": 7, "title": "Dune"}}], "found": 1}}}
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_sort_fallback(monkeypatch):
|
||||
"""Keep the process-wide sort fallback from leaking between tests."""
|
||||
monkeypatch.setattr("shelfmark.metadata_providers.hardcover._sort_fallback_until", 0.0)
|
||||
|
||||
|
||||
def _reject_sorted(calls: list[dict[str, Any]], *, success=ONE_HIT):
|
||||
"""Build an _execute_query stand-in that rejects any request carrying a sort."""
|
||||
|
||||
def fake_execute(query: str, variables):
|
||||
calls.append(dict(variables))
|
||||
return REJECTED if variables.get("sort") else success
|
||||
|
||||
return fake_execute
|
||||
|
||||
|
||||
class TestHardcoverSortFallback:
|
||||
def test_retries_without_sort_when_hardcover_rejects_the_sort(self, monkeypatch):
|
||||
provider = HardcoverProvider(api_key="test-token")
|
||||
calls: list[dict[str, Any]] = []
|
||||
monkeypatch.setattr(provider, "_execute_query", _reject_sorted(calls))
|
||||
|
||||
result = provider._execute_search_query("query", {"query": "dune", "sort": "relevance"})
|
||||
|
||||
assert result == ONE_HIT
|
||||
assert [call["sort"] for call in calls] == ["relevance", ""]
|
||||
|
||||
def test_treats_an_empty_result_set_as_success(self, monkeypatch):
|
||||
provider = HardcoverProvider(api_key="test-token")
|
||||
calls: list[dict[str, Any]] = []
|
||||
monkeypatch.setattr(
|
||||
provider, "_execute_query", lambda query, variables: calls.append(variables) or EMPTY
|
||||
)
|
||||
|
||||
result = provider._execute_search_query("query", {"query": "dune", "sort": "rating:desc"})
|
||||
|
||||
assert result == EMPTY
|
||||
assert len(calls) == 1
|
||||
|
||||
def test_reports_failure_when_the_retry_is_also_rejected(self, monkeypatch):
|
||||
provider = HardcoverProvider(api_key="test-token")
|
||||
calls: list[dict[str, Any]] = []
|
||||
monkeypatch.setattr(
|
||||
provider, "_execute_query", lambda query, variables: calls.append(variables) or REJECTED
|
||||
)
|
||||
|
||||
result = provider._execute_search_query("query", {"query": "dune", "sort": "rating:desc"})
|
||||
|
||||
assert result is None
|
||||
assert len(calls) == 2
|
||||
|
||||
def test_reports_failure_for_an_unsorted_rejection(self, monkeypatch):
|
||||
provider = HardcoverProvider(api_key="test-token")
|
||||
calls: list[dict[str, Any]] = []
|
||||
monkeypatch.setattr(
|
||||
provider, "_execute_query", lambda query, variables: calls.append(variables) or REJECTED
|
||||
)
|
||||
|
||||
result = provider._execute_search_query("query", {"query": "dune", "sort": ""})
|
||||
|
||||
assert result is None
|
||||
assert len(calls) == 1
|
||||
|
||||
def test_skips_the_doomed_request_on_later_searches(self, monkeypatch):
|
||||
provider = HardcoverProvider(api_key="test-token")
|
||||
calls: list[dict[str, Any]] = []
|
||||
monkeypatch.setattr(provider, "_execute_query", _reject_sorted(calls))
|
||||
|
||||
provider._execute_search_query("query", {"query": "dune", "sort": "relevance"})
|
||||
provider._execute_search_query("query", {"query": "hyperion", "sort": "relevance"})
|
||||
|
||||
assert [call["sort"] for call in calls] == ["relevance", "", ""]
|
||||
|
||||
def test_search_returns_results_despite_a_rejected_sort(self, monkeypatch):
|
||||
provider = HardcoverProvider(api_key="test-token")
|
||||
calls: list[dict[str, Any]] = []
|
||||
monkeypatch.setattr(provider, "_execute_query", _reject_sorted(calls))
|
||||
|
||||
result = provider.search_paginated(
|
||||
MetadataSearchOptions(query="dune sort fallback", page=1, limit=25)
|
||||
)
|
||||
|
||||
assert result.total_found == 1
|
||||
assert [book.title for book in result.books] == ["Dune"]
|
||||
assert [call["sort"] for call in calls] == ["_text_match:desc,users_count:desc", ""]
|
||||
|
||||
|
||||
class TestSearchPayloadRejection:
|
||||
def test_distinguishes_a_null_body_from_an_empty_result_set(self):
|
||||
from shelfmark.metadata_providers.hardcover import _search_payload_rejected
|
||||
|
||||
assert _search_payload_rejected(REJECTED) is True
|
||||
assert _search_payload_rejected(EMPTY) is False
|
||||
assert _search_payload_rejected(ONE_HIT) is False
|
||||
assert _search_payload_rejected(None) is False
|
||||
assert _search_payload_rejected({}) is False
|
||||
# Non-search payloads (list lookups, book fetches) must pass through.
|
||||
assert _search_payload_rejected({"series": [{"id": 1}]}) is False
|
||||
Reference in New Issue
Block a user