Google Books: retry transient errors, and tell a failed search from an empty one

A failed metadata search is reported to the user as "No Result(s) found!
Please try another keyword." — the same words as a genuine miss, so the user
goes looking for a keyword problem that is not there. search() swallows every
exception and returns [], which the route cannot tell from an empty result.

Three ways it happens:

1. No API key, which is the default. An unkeyed Books call is billed to a
   shared anonymous Google project whose daily quota is permanently spent:
   429 RESOURCE_EXHAUSTED, "Quota exceeded ... for consumer
   project_number:<redacted>". The same project number comes back from two
   unrelated networks, so it is shared, not per-IP. Every stock install with
   no key therefore gets nothing, and is told its keyword is wrong.

2. Transient 5xx with a valid key: 2 of 12 single-shot keyed requests
   answered 503. There is no retry anywhere in cps/.

3. No request timeout, so a stalled connection holds the search thread until
   the client gives up. /metadata/search waits on every provider.

Changes:

- google.py retries 500/502/503/504 (3 retries, 0.3 backoff) and passes a
  (5, 15) timeout. 429 is deliberately not retried: a spent daily quota does
  not clear, so it must fail fast.
- google.py returns None on failure and [] on an empty result. The signature
  is already Optional[List[MetaRecord]] and callers already handle None.
- search_metadata.py answers 502 when every provider it asked failed.
  get_meta.js already tells msg.search_error from msg.no_result; it had no
  way to know which had happened. "Search error!" is an existing string,
  translated in every locale — no new text, no JS change.
- search_metadata.py no longer lets one provider's exception fail the whole
  request, which used to discard every other provider's results.
- The API key is read per search instead of at import, so saving one in the
  admin page takes effect without a restart.

allowed_methods is left at the urllib3 default, which already covers GET;
naming it would need urllib3 >= 1.26 and requirements.txt allows 1.22. The
Session is built per call because requests.Session is not thread-safe and the
providers are searched in a thread pool.

Not included, happy to add: cps/admin.py still sets reboot_required for
config_googlebooks_api_key, now unnecessary; and the other five providers
still return [] on failure.
This commit is contained in:
Jack Halford
2026-09-02 01:03:22 +02:00
parent 1b796ed803
commit a32f2783c0
2 changed files with 36 additions and 4 deletions
+20 -3
View File
@@ -22,6 +22,8 @@ from urllib.parse import quote
from datetime import datetime
import requests
from requests.adapters import HTTPAdapter
from urllib3.util.retry import Retry
from cps import logger, config
from cps.isoLanguages import get_lang3, get_language_name
@@ -29,6 +31,13 @@ from cps.services.Metadata import MetaRecord, MetaSourceInfo, Metadata
log = logger.create()
# Google answers a share of requests with a transient 5xx, which is otherwise
# indistinguishable from "not found". raise_on_status lets the last response
# through so it is logged with its status instead of as a MaxRetryError.
RETRY = Retry(total=3, backoff_factor=0.3, status_forcelist=(500, 502, 503, 504),
raise_on_status=False)
TIMEOUT = (5, 15)
class Google(Metadata):
__name__ = "Google Books"
@@ -38,7 +47,6 @@ class Google(Metadata):
BOOK_URL = "https://books.google.com/books?id="
SEARCH_URL = "https://www.googleapis.com/books/v1/volumes?q="
ISBN_TYPE = "ISBN_13"
API_KEY = "&key=" + config.config_googlebooks_api_key
def search(
self, query: str, generic_cover: str = "", locale: str = "en"
@@ -50,12 +58,21 @@ class Google(Metadata):
if title_tokens:
tokens = [quote(t.encode("utf-8")) for t in title_tokens]
query = "+".join(tokens)
# Read per search, so a key saved in the admin page takes effect
# without a restart.
key = config.config_googlebooks_api_key
try:
results = requests.get(Google.SEARCH_URL + query + Google.API_KEY)
# A session per call: Session is not thread-safe and the
# providers are searched in a thread pool.
session = requests.Session()
session.mount("https://", HTTPAdapter(max_retries=RETRY))
results = session.get(
Google.SEARCH_URL + query + ("&key=" + key if key else ""),
timeout=TIMEOUT)
results.raise_for_status()
except Exception as e:
log.warning(e)
return []
return None # None is a failed search, [] is an empty one
for result in results.json().get("items", []):
val.append(
self._parse_search_result(
+16 -1
View File
@@ -157,6 +157,7 @@ def metadata_search():
data = list()
active = current_user.view_settings.get("metadata", {})
locale = get_locale()
asked = failed = 0
if query:
static_cover = url_for("static", filename="generic_cover.jpg")
# ret = cl[0].search(query, static_cover, locale)
@@ -166,6 +167,20 @@ def metadata_search():
for c in cl
if active.get(c.__id__, True)
}
asked = len(meta)
for future in concurrent.futures.as_completed(meta):
data.extend(_serialize_metadata_records(future.result()))
try:
records = future.result()
except Exception as e:
# One provider raising used to fail the whole request.
log.warning("Metadata provider %s failed: %s", meta[future].__id__, e)
records = None
if records is None: # failed, as opposed to found nothing
failed += 1
continue
data.extend(_serialize_metadata_records(records))
# Nothing to say about the keyword if every source failed, so 502 makes the
# client show "Search error!" rather than "No Result(s) found!".
if asked and failed == asked:
return make_response(jsonify(data), 502)
return make_response(jsonify(data))