From 4654b903c9311b8b975e6794ff2e536222203fbf Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Fri, 20 Feb 2026 20:52:02 +0530 Subject: [PATCH] remote: fix StoreObjectNotFound exception lost over RPC, fixes #9380 --- src/borg/cache.py | 9 +++----- src/borg/remote.py | 16 +++++++++++--- src/borg/testsuite/cache_test.py | 31 ++++++++++++++++++++++++++- src/borg/testsuite/repository_test.py | 8 ++++++- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/borg/cache.py b/src/borg/cache.py index 30ff80e9f0..6d08a92927 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -712,8 +712,7 @@ def delete_chunkindex_cache(repository): cache_name = f"cache/chunks.{hash}" try: repository.store_delete(cache_name) - except (Repository.ObjectNotFound, StoreObjectNotFound): - # TODO: ^ seem like RemoteRepository raises Repository.ONF instead of StoreONF + except StoreObjectNotFound: pass logger.debug(f"cached chunk indexes deleted: {hashes}") @@ -769,8 +768,7 @@ def write_chunkindex_to_repo_cache( cache_name = f"cache/chunks.{hash}" try: repository.store_delete(cache_name) - except (Repository.ObjectNotFound, StoreObjectNotFound): - # TODO: ^ seem like RemoteRepository raises Repository.ONF instead of StoreONF + except StoreObjectNotFound: pass if delete_these: logger.debug(f"cached chunk indexes deleted: {delete_these}") @@ -782,8 +780,7 @@ def read_chunkindex_from_repo_cache(repository, hash): logger.debug(f"trying to load {cache_name} from the repo...") try: chunks_data = repository.store_load(cache_name) - except (Repository.ObjectNotFound, StoreObjectNotFound): - # TODO: ^ seem like RemoteRepository raises Repository.ONF instead of StoreONF + except StoreObjectNotFound: logger.debug(f"{cache_name} not found in the repository.") else: if xxh64(chunks_data, seed=CHUNKINDEX_HASH_SEED) == hex_to_bin(hash): diff --git a/src/borg/remote.py b/src/borg/remote.py index f57c2a1406..927c1a3c9b 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -35,7 +35,7 @@ from .manifest import NoManifestError from .helpers import msgpack from .legacyrepository import LegacyRepository -from .repository import Repository +from .repository import Repository, StoreObjectNotFound from .version import parse_version, format_version from .checksums import xxh64 from .helpers.datastruct import EfficientCollectionQueue @@ -269,11 +269,17 @@ def inner_serve(): logging.debug("\n".join(ex_full)) sys_info = sysinfo() + # StoreObjectNotFound and Repository.ObjectNotFound both have + # __name__ == "ObjectNotFound", so we need to distinguish them + # explicitly for correct client-side reconstruction. + exc_cls_name = ( + "StoreObjectNotFound" if isinstance(e, StoreObjectNotFound) else e.__class__.__name__ + ) try: msg = msgpack.packb( { MSGID: msgid, - "exception_class": e.__class__.__name__, + "exception_class": exc_cls_name, "exception_args": e.args, "exception_full": ex_full, "exception_short": ex_short, @@ -285,7 +291,7 @@ def inner_serve(): msg = msgpack.packb( { MSGID: msgid, - "exception_class": e.__class__.__name__, + "exception_class": exc_cls_name, "exception_args": [ x if isinstance(x, (str, bytes, int)) else None for x in e.args ], @@ -403,6 +409,8 @@ def inject_exception(self, kind): raise PathNotAllowed("foo") elif kind == "ObjectNotFound": raise self.RepoCls.ObjectNotFound(s1, s2) + elif kind == "StoreObjectNotFound": + raise StoreObjectNotFound(s1) elif kind == "InvalidRPCMethod": raise InvalidRPCMethod(s1) elif kind == "divide": @@ -784,6 +792,8 @@ def handle_error(unpacked): raise Repository.ParentPathDoesNotExist(args[0]) elif error == "ObjectNotFound": raise Repository.ObjectNotFound(args[0], self.location.processed) + elif error == "StoreObjectNotFound": + raise StoreObjectNotFound(args[0]) elif error == "InvalidRPCMethod": raise InvalidRPCMethod(args[0]) elif error == "LockTimeout": diff --git a/src/borg/testsuite/cache_test.py b/src/borg/testsuite/cache_test.py index e6a5092ffa..86a7d5b746 100644 --- a/src/borg/testsuite/cache_test.py +++ b/src/borg/testsuite/cache_test.py @@ -5,7 +5,7 @@ from .hashindex_test import H from .crypto.key_test import TestKey from ..archive import Statistics -from ..cache import AdHocWithFilesCache +from ..cache import AdHocWithFilesCache, delete_chunkindex_cache, read_chunkindex_from_repo_cache from ..crypto.key import AESOCBRepoKey from ..manifest import Manifest from ..repository import Repository @@ -53,3 +53,32 @@ def test_files_cache(self, cache): assert cache.file_known_and_unchanged(b"foo", bytes(32), st) == (False, None) assert cache.cache_mode == "d" assert cache.files == {} + + +def test_delete_chunkindex_cache_missing(tmp_path): + """delete_chunkindex_cache handles StoreObjectNotFound when cache entries do not exist.""" + from borgstore.store import ObjectNotFound as StoreObjectNotFound + + repository_location = os.fspath(tmp_path / "repository") + with Repository(repository_location, exclusive=True, create=True) as repository: + # Create a cache entry so list_chunkindex_hashes finds it. + repository.store_store(f"cache/chunks.{'a' * 64}", b"data") + # Patch store_delete to raise StoreObjectNotFound (simulates a race or already-deleted entry). + original_store_delete = repository.store_delete + + def failing_store_delete(name): + raise StoreObjectNotFound(name) + + repository.store_delete = failing_store_delete + # Should not raise — the except StoreObjectNotFound catches it. + delete_chunkindex_cache(repository) + repository.store_delete = original_store_delete + + +def test_read_chunkindex_from_repo_cache_missing(tmp_path): + """read_chunkindex_from_repo_cache handles StoreObjectNotFound when cache does not exist.""" + repository_location = os.fspath(tmp_path / "repository") + with Repository(repository_location, exclusive=True, create=True) as repository: + # Try to load a non-existent cache entry — should return None, not raise. + result = read_chunkindex_from_repo_cache(repository, "f" * 64) + assert result is None diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index fc1d9efec5..67afa65843 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -9,7 +9,7 @@ from ..helpers import IntegrityError from ..platformflags import is_win32 from ..remote import RemoteRepository, InvalidRPCMethod, PathNotAllowed -from ..repository import Repository, MAX_DATA_SIZE +from ..repository import Repository, StoreObjectNotFound, MAX_DATA_SIZE from ..repoobj import RepoObj from .hashindex_test import H @@ -214,6 +214,12 @@ def test_remote_rpc_exception_transport(remote_repository): assert e.args[0] == s1 assert e.args[1] == remote_repository.location.processed + try: + remote_repository.call("inject_exception", {"kind": "StoreObjectNotFound"}) + except StoreObjectNotFound as e: + assert len(e.args) == 1 + assert e.args[0] == s1 + try: remote_repository.call("inject_exception", {"kind": "InvalidRPCMethod"}) except InvalidRPCMethod as e: