diff --git a/src/cachew/__init__.py b/src/cachew/__init__.py index 13ad94c..76710a4 100644 --- a/src/cachew/__init__.py +++ b/src/cachew/__init__.py @@ -523,6 +523,7 @@ def cachew_wrapper[**P, ItemT]( # see test_recursive* early_exit = False running_uncached = False + served_from_cache = False try: BackendCls = BACKENDS[C.backend] @@ -549,6 +550,7 @@ def cachew_wrapper[**P, ItemT]( if new_hash == old_hash: logger.debug('hash matched: loading from cache') yield from session.cached_items() + served_from_cache = True return logger.debug('hash mismatch: computing data and writing to db') @@ -577,22 +579,31 @@ def cachew_wrapper[**P, ItemT]( try: yield from session.write_to_cache(func(*args, **kwargs)) except GeneratorExit: + # GeneratorExit itself is not caught below, but SQLAlchemy cleanup during interpreter shutdown can raise a normal Exception while unwinding. early_exit = True raise except CacheReadError: # Cache read failures bypass THROW_ON_ERROR because fallback can duplicate already-yielded cached items. + # This can be thrown from session.cached_items() raise except Exception as e: if running_uncached: raise - # sigh... see test_early_exit_shutdown... + # Work around known SQLAlchemy/sqlite shutdown noise; do not suppress other cleanup errors. + # See test_early_exit_shutdown. if early_exit and 'Cannot operate on a closed database' in str(e): return - # todo hmm, kinda annoying that it tries calling the function twice? - # but gonna require some sophisticated cooperation with the cached wrapper otherwise cachew_error(e, logger=logger) + + if served_from_cache: + # this can happen if we fully read from the cache, but hit some error while shutting backend down + # - we're past reading, so we emitted all items user wanted from cache + # - we don't want to yield any items from original func + # so it's safe to simply return + return + yield from func(*args, **kwargs) diff --git a/src/cachew/tests/test_cachew.py b/src/cachew/tests/test_cachew.py index 8c72f84..33a1574 100644 --- a/src/cachew/tests/test_cachew.py +++ b/src/cachew/tests/test_cachew.py @@ -1123,6 +1123,43 @@ def fun() -> Iterator[Item]: assert calls == 1 +def test_cache_hit_cleanup_error_after_full_read_does_not_fallback( + tmp_path: Path, + restore_settings, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ + If cache cleanup fails after a full cache hit, cachew must not fallback and re-emit fresh items. + """ + from .. import BACKENDS + + settings.THROW_ON_ERROR = False + + calls = 0 + + @cachew(tmp_path) + def fun() -> Iterator[int]: + nonlocal calls + calls += 1 + yield 1 + yield 2 + + assert list(fun()) == [1, 2] + assert calls == 1 + + BackendCls = BACKENDS[settings.DEFAULT_BACKEND] + + class CleanupErrorBackend(BackendCls): # type: ignore[valid-type, misc] + def __exit__(self, *exc_info) -> None: + super().__exit__(*exc_info) + raise RuntimeError('post-cache cleanup failed') + + monkeypatch.setitem(BACKENDS, settings.DEFAULT_BACKEND, CleanupErrorBackend) + + assert list(fun()) == [1, 2] + assert calls == 1 + + def test_locked_write_uncached_exception_propagates_without_retry( tmp_path: Path, restore_settings,