Skip to content

Commit 6a5dd9e

Browse files
committed
refactor: Log sync persistent-store close errors instead of returning them
Store.close() previously returned the close error as Optional[Exception], which the only caller (FDv2.stop) discarded, so a failed close was silently lost. It now logs a warning and returns None. Closing happens at shutdown, where there is no caller left to react to the error.
1 parent 064ae37 commit 6a5dd9e

2 files changed

Lines changed: 24 additions & 3 deletions

File tree

ldclient/impl/datasystem/store.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ def __mapping(data: Dict[str, ModelEntity]) -> Dict[str, Dict[str, Any]]:
521521
return e
522522
return None
523523

524-
def close(self) -> Optional[Exception]:
524+
def close(self) -> None:
525525
"""Close the store and any persistent store if configured."""
526526
with self._lock:
527527
if self._persistent_store is not None:
@@ -532,8 +532,7 @@ def close(self) -> Optional[Exception]:
532532
if callable(close):
533533
close()
534534
except Exception as e:
535-
return e
536-
return None
535+
log.warning("Error closing the persistent store: %s", e)
537536

538537
def get_data_store_status_provider(self) -> Optional[DataStoreStatusProvider]:
539538
"""Get the data store status provider for the persistent store, if configured."""

ldclient/testing/impl/datasystem/test_fdv2_persistence.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,3 +845,25 @@ def test_variation_does_not_throw_when_persistent_store_errors_during_warm_start
845845
for record in caplog.records
846846
if record.levelname == "ERROR"
847847
)
848+
849+
850+
def test_persistent_store_close_logs_and_swallows_error(caplog):
851+
"""A persistent-store close error is logged as a warning, not raised."""
852+
from ldclient.impl.datasystem.store import Store
853+
from ldclient.impl.listeners import Listeners
854+
855+
class ClosingFailsStore(StubFeatureStore):
856+
def close(self):
857+
raise RuntimeError("close boom")
858+
859+
store = Store(Listeners(), Listeners())
860+
store.with_persistence(ClosingFailsStore(), True, None)
861+
862+
# close() must log the error rather than raise it.
863+
store.close()
864+
865+
assert any(
866+
"Error closing the persistent store" in record.message
867+
for record in caplog.records
868+
if record.levelname == "WARNING"
869+
)

0 commit comments

Comments
 (0)