Skip to content

Commit 2f64de2

Browse files
committed
core: handle possible issue with duplicate items during error while backend shuts down
1 parent 7d2e3ce commit 2f64de2

2 files changed

Lines changed: 51 additions & 3 deletions

File tree

src/cachew/__init__.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ def cachew_wrapper[**P, ItemT](
523523
# see test_recursive*
524524
early_exit = False
525525
running_uncached = False
526+
served_from_cache = False
526527
try:
527528
BackendCls = BACKENDS[C.backend]
528529

@@ -549,6 +550,7 @@ def cachew_wrapper[**P, ItemT](
549550
if new_hash == old_hash:
550551
logger.debug('hash matched: loading from cache')
551552
yield from session.cached_items()
553+
served_from_cache = True
552554
return
553555

554556
logger.debug('hash mismatch: computing data and writing to db')
@@ -577,22 +579,31 @@ def cachew_wrapper[**P, ItemT](
577579
try:
578580
yield from session.write_to_cache(func(*args, **kwargs))
579581
except GeneratorExit:
582+
# GeneratorExit itself is not caught below, but SQLAlchemy cleanup during interpreter shutdown can raise a normal Exception while unwinding.
580583
early_exit = True
581584
raise
582585
except CacheReadError:
583586
# Cache read failures bypass THROW_ON_ERROR because fallback can duplicate already-yielded cached items.
587+
# This can be thrown from session.cached_items()
584588
raise
585589
except Exception as e:
586590
if running_uncached:
587591
raise
588592

589-
# sigh... see test_early_exit_shutdown...
593+
# Work around known SQLAlchemy/sqlite shutdown noise; do not suppress other cleanup errors.
594+
# See test_early_exit_shutdown.
590595
if early_exit and 'Cannot operate on a closed database' in str(e):
591596
return
592597

593-
# todo hmm, kinda annoying that it tries calling the function twice?
594-
# but gonna require some sophisticated cooperation with the cached wrapper otherwise
595598
cachew_error(e, logger=logger)
599+
600+
if served_from_cache:
601+
# this can happen if we fully read from the cache, but hit some error while shutting backend down
602+
# - we're past reading, so we emitted all items user wanted from cache
603+
# - we don't want to yield any items from original func
604+
# so it's safe to simply return
605+
return
606+
596607
yield from func(*args, **kwargs)
597608

598609

src/cachew/tests/test_cachew.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,6 +1123,43 @@ def fun() -> Iterator[Item]:
11231123
assert calls == 1
11241124

11251125

1126+
def test_cache_hit_cleanup_error_after_full_read_does_not_fallback(
1127+
tmp_path: Path,
1128+
restore_settings,
1129+
monkeypatch: pytest.MonkeyPatch,
1130+
) -> None:
1131+
"""
1132+
If cache cleanup fails after a full cache hit, cachew must not fallback and re-emit fresh items.
1133+
"""
1134+
from .. import BACKENDS
1135+
1136+
settings.THROW_ON_ERROR = False
1137+
1138+
calls = 0
1139+
1140+
@cachew(tmp_path)
1141+
def fun() -> Iterator[int]:
1142+
nonlocal calls
1143+
calls += 1
1144+
yield 1
1145+
yield 2
1146+
1147+
assert list(fun()) == [1, 2]
1148+
assert calls == 1
1149+
1150+
BackendCls = BACKENDS[settings.DEFAULT_BACKEND]
1151+
1152+
class CleanupErrorBackend(BackendCls):
1153+
def __exit__(self, *exc_info) -> None:
1154+
super().__exit__(*exc_info)
1155+
raise RuntimeError('post-cache cleanup failed')
1156+
1157+
monkeypatch.setitem(BACKENDS, settings.DEFAULT_BACKEND, CleanupErrorBackend)
1158+
1159+
assert list(fun()) == [1, 2]
1160+
assert calls == 1
1161+
1162+
11261163
def test_locked_write_uncached_exception_propagates_without_retry(
11271164
tmp_path: Path,
11281165
restore_settings,

0 commit comments

Comments
 (0)