fix(library): back off after a failed index instead of retrying for every result

shelfmark/core/library_index.py

@@ -37,13 +37,17 @@
 logger = setup_logger(__name__)
 
 _CACHE_TTL_SECONDS = 600  # Re-index a library at most every 10 minutes unless it changed.
+# After a failed index, answer from the stale cache for this long before trying again, so a
+# search page with dozens of results costs one failed attempt and one warning, not one each.
+_FAILURE_BACKOFF_SECONDS = 60
 
 
 @dataclass
 class _CacheSlot:
     entries: list[LibraryEntry] | None = None
     fetched_at: float = 0.0
     fingerprint: object | None = None
+    failed_at: float | None = None
 
 
 _lock = threading.Lock()
@@ -61,11 +65,17 @@ def _store(provider_name: str, entries: list[LibraryEntry], fingerprint: object
         slot.entries = entries
         slot.fetched_at = time.monotonic()
         slot.fingerprint = fingerprint
+        slot.failed_at = None
 
 
 def _entries_for(provider: LibraryProvider) -> list[LibraryEntry]:
     """Cached entries for one provider, re-indexed past the TTL or when the library changed."""
     slot = _slot(provider.name)
+    with _lock:
+        if slot.failed_at is not None and time.monotonic() - slot.failed_at < (
+            _FAILURE_BACKOFF_SECONDS
+        ):
+            return slot.entries or []
     try:
         fingerprint = provider.fingerprint()
         with _lock:
@@ -78,6 +88,7 @@ def _entries_for(provider: LibraryProvider) -> list[LibraryEntry]:
     except Exception as exc:  # noqa: BLE001 - any failure must fail open
         logger.warning("library check: %s unavailable (%s); failing open", provider.describe(), exc)
         with _lock:
+            slot.failed_at = time.monotonic()
             return slot.entries or []  # Use the stale cache if we have one.
 
     _store(provider.name, entries, fingerprint)

tests/core/test_library_index.py

@@ -234,6 +234,53 @@ def test_provider_error_keeps_answering_from_the_stale_cache(
     assert len(warnings) == 1
 
 
+def test_a_failing_library_is_tried_once_per_search_not_once_per_result(
+    providers: list[_Provider], monkeypatch: pytest.MonkeyPatch, clock: list[float]
+) -> None:
+    provider = _Provider("ebook", {"ebook"}, [_DCC_ENTRY])
+    providers.append(provider)
+    assert library_index.is_in_library(_book(), "ebook") is True
+
+    provider.error = RuntimeError("boom")
+    clock[0] += library_index._CACHE_TTL_SECONDS + 1
+    warnings = _warnings(monkeypatch)
+
+    for _ in range(40):  # one search page, every result asks
+        assert library_index.is_in_library(_book(), "ebook") is True
+
+    assert provider.fetches == 2  # the first index, then one failed attempt
+    assert len(warnings) == 1
+
+
+def test_a_failing_library_with_no_cache_is_not_retried_for_every_result(
+    providers: list[_Provider], clock: list[float]
+) -> None:
+    provider = _Provider("ebook", {"ebook"}, error=OSError("no such file"))
+    providers.append(provider)
+
+    for _ in range(10):
+        assert library_index.is_in_library(_book(), "ebook") is False
+
+    assert provider.fetches == 1
+
+
+def test_a_failing_library_is_retried_once_the_back_off_has_passed(
+    providers: list[_Provider], clock: list[float]
+) -> None:
+    provider = _Provider("ebook", {"ebook"}, [_DCC_ENTRY], error=OSError("down"))
+    providers.append(provider)
+    assert library_index.is_in_library(_book(), "ebook") is False
+
+    provider.error = None
+    clock[0] += library_index._FAILURE_BACKOFF_SECONDS - 1
+    assert library_index.is_in_library(_book(), "ebook") is False
+    assert provider.fetches == 1
+
+    clock[0] += 2
+    assert library_index.is_in_library(_book(), "ebook") is True
+    assert provider.fetches == 2
+
+
 def test_entries_are_cached_until_the_ttl_expires(
     providers: list[_Provider], clock: list[float]
 ) -> None: