diff --git a/docs/changes.rst b/docs/changes.rst index d3139fa75a..f2684b737b 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -197,6 +197,13 @@ Fixes: packs, #9443 - --find-lost-archives reuses the archive metadata ids found by --verify-data, #8478 - --repair rebuilds a corrupt index only once, in the archives phase, #10434 + - do not crash with a traceback when a repository object can not be read + (I/O error, e.g. failing disk or flaky network filesystem). The affected object + is reported, the check continues and fails at the end. Such an object is not + recorded as corrupt (a later check verifies it again), ``--repair`` refuses to + repair around it and ``--verify-data`` no longer deletes chunks it could not + read. Other commands stop with the read error instead of working with a + partially readable repository, #3509 - extract --continue: skip complete dirs, count skipped files in --progress, #6857 - create: back up automounted directories, #6652 - security: same local repo dir via another path is not a relocation, #8188 diff --git a/docs/internals/frontends.rst b/docs/internals/frontends.rst index 3140b7514a..af3fb1c06e 100644 --- a/docs/internals/frontends.rst +++ b/docs/internals/frontends.rst @@ -877,6 +877,10 @@ Errors Repository {} needs its key to access the store object {}, but no key was set. Repository.LegacyRepository rc: 29 traceback: no {} looks like a borg 1.x repository, use --from-borg1 to access it (e.g. with borg transfer). + Repository.RepairUnsafe rc: 34 traceback: no + Not repairing: {} repository object(s) could not be read. Fix the underlying problem, then check again. + Repository.StoreReadError rc: 35 traceback: no + Error reading {} from the repository: {}. Check the storage hardware / filesystem. MandatoryFeatureUnsupported rc: 25 traceback: no Unsupported repository feature(s) {}. A newer version of Borg is required to access this repository. diff --git a/docs/usage/check.rst.inc b/docs/usage/check.rst.inc index 8b4378b8e6..4a03085aa8 100644 --- a/docs/usage/check.rst.inc +++ b/docs/usage/check.rst.inc @@ -210,6 +210,24 @@ would report them as lost. After a ``--repair`` that stored or deleted chunks, b packs the repair wrote, makes the chunk index match them and stores it; that always runs to completion, also after a Ctrl-C. +Unreadable repository objects ++++++++++++++++++++++++++++++ + +A repository object that cannot be read at all (an I/O error from a failing disk, a +flaky network filesystem, ...) is different from a corrupt one: Borg never saw its +content, so it cannot tell whether the object is fine. Such an object is reported and +the check continues, so one run lists everything that is affected; the check then +fails. The object is not remembered as corrupt, so a later check verifies it again. + +Because the data may well be readable again once the underlying problem is fixed, +``--repair`` refuses to repair while there are unreadable objects, and +``--verify-data`` leaves chunks it could not read in place instead of deleting them. +Fix the storage hardware, filesystem or network first, then run the check again. + +Only ``borg check`` goes on like this. Other commands stop with the read error rather +than work with a partially readable repository - e.g. ``borg prune`` would otherwise +see an archive whose metadata it could not read as undated and thus as the oldest one. + About repair mode +++++++++++++++++ diff --git a/src/borg/archive.py b/src/borg/archive.py index 00a32902eb..f7f360f09b 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -2607,7 +2607,8 @@ def verify_data(self): verified = 0 # chunks actually verified defect_chunks = [] archive_meta_ids = set() - untrusted_pack_ids = set() # ids of the packs that are missing or hold a defect chunk + untrusted_pack_ids = set() # ids of the packs that are missing, unreadable or hold a defect chunk + unreadable_chunks = 0 # chunks we could not even read, refs #3509 pi = ProgressIndicatorPercent( total=chunks_count, msg="Verifying data %6.2f%%", step=0.01, msgid="check.verify_data" ) @@ -2652,6 +2653,21 @@ def verify_data(self): for missing_id in chunk_ids: logger.debug("chunk %s: pack %s is missing.", bin_to_hex(missing_id), bin_to_hex(pack_id)) pi.show(increase=lost) + except Repository.StoreReadError as err: + # the pack could not be read (I/O error), and like above, get_many() raises before any chunk + # of this pack was read. unreadable, not defect: we did not see the content, so we must not + # conclude it is bad and delete it - it may read fine once the hardware/fs problem is fixed, + # refs #3509. + self.error_found = True + untrusted_pack_ids.add(pack_id) # like a missing pack: its objects may be unreadable + unreadable = len(chunk_ids) + errors += unreadable + unreadable_chunks += unreadable + verified += unreadable # they are not read, but they are accounted for, like a failed read + logger.error("%s, %d chunks could not be verified.", err, unreadable) + for unreadable_id in chunk_ids: + logger.debug("chunk %s: pack %s could not be read.", bin_to_hex(unreadable_id), bin_to_hex(pack_id)) + pi.show(increase=unreadable) # this pack is done: drop it, the cache would else keep PACK_READER_CACHE_SIZE whole packs # of which none is read again, and the retries below must really re-read from the store. self.repository.clear_pack_cache() @@ -2684,6 +2700,11 @@ def verify_data(self): ro_type=ROBJ_DONTCARE, assert_id_place="verify_data", ) + except Repository.StoreReadError as err: + # the retry did not fail on the content, it failed to read at all: keep the + # chunk, the read may well succeed again later, refs #3509. + unreadable_chunks += 1 + logger.error("chunk %s not deleted, could not be re-read: %s", bin_to_hex(defect_chunk), err) except IntegrityErrorBase: # failed twice -> remove this defect chunk. delete rewrites its pack without it, # keeping the other chunks, and removes it from self.chunks, so rebuild_archives @@ -2717,18 +2738,23 @@ def verify_data(self): if not sig_int: # the ids of an interrupted pass are incomplete. self.archive_meta_ids = archive_meta_ids + if unreadable_chunks: + logger.error( + "%d chunk(s) could not be read and were left untouched. This usually means a hardware, " + "filesystem or network problem - fix that first, then run the check again.", + unreadable_chunks, + ) log = logger.error if errors else logger.info if sig_int: log( - "Interrupted cryptographic data integrity verification, " - "verified %d of %d chunks with %d integrity errors.", + "Interrupted cryptographic data integrity verification, verified %d of %d chunks with %d error(s).", verified, chunks_count, errors, ) else: log( - "Finished cryptographic data integrity verification, verified %d chunks with %d integrity errors.", + "Finished cryptographic data integrity verification, verified %d chunks with %d error(s).", verified, errors, ) @@ -2818,13 +2844,19 @@ def check_archive_meta(chunk_id): """Load the archive metadata object chunk_id. If it has no archives directory entry, create one (with --repair) or log that it would create one. """ - cdata = self.repository.get(chunk_id) try: + cdata = self.repository.get(chunk_id) meta, data = self.repo_objs.parse(chunk_id, cdata, ro_type=ROBJ_DONTCARE) except IntegrityErrorBase as exc: logger.error("Skipping corrupted chunk: %s", exc) self.error_found = True return + except Repository.StoreReadError as exc: + # unreadable, not corrupt: this scan only looks for archive metadata, so skipping + # such a chunk can at worst miss a lost archive - it never drops data, refs #3509. + logger.error("Skipping unreadable chunk: %s", exc) + self.error_found = True + return if meta["type"] != ROBJ_ARCHIVE_META: return # should never happen try: @@ -2879,6 +2911,11 @@ def check_archive_meta(chunk_id): logger.error("Skipping corrupted chunk: %s", exc) self.error_found = True continue + except Repository.StoreReadError as exc: + # see check_archive_meta. + logger.error("Skipping unreadable chunk: %s", exc) + self.error_found = True + continue if meta["type"] != ROBJ_ARCHIVE_META: continue check_archive_meta(chunk_id) @@ -3123,6 +3160,9 @@ def robust_item_ids(): newest=newest, older=older, newer=newer, + # an archive whose metadata we can not read gets a placeholder entry here, so the + # other archives still get checked; the loop below reports it, refs #3509. + tolerate_read_errors=True, ) if match and not archive_infos: logger.warning("--match-archives %s does not match any archives", match) @@ -3131,7 +3171,7 @@ def robust_item_ids(): if last and len(archive_infos) < last: logger.warning("--last %d archives: only found %d archives", last, len(archive_infos)) else: - archive_infos = self.manifest.archives.list(sort_by=sort_by) + archive_infos = self.manifest.archives.list(sort_by=sort_by, tolerate_read_errors=True) num_archives = len(archive_infos) if deadline is not None: # archives without an ok record first, then the least recently checked ones, so that @@ -3170,10 +3210,11 @@ def check_order(info): formatted = formatter.format_item(info, jsonline=False) # the formatter uses defaults for keys like {comment} if it has no archive metadata. metadata_missing = bool(formatter.used_call_keys) and formatter.archive is None - except (Repository.ObjectNotFound, IntegrityErrorBase): + except (Repository.ObjectNotFound, IntegrityErrorBase, Repository.StoreReadError): metadata_missing = True if metadata_missing: - # keys like {comment} need the archive metadata, which is damaged or missing here. + # keys like {comment} need the archive metadata, which is damaged, missing or + # unreadable here. # use the values from the archive directory entry, they are always available. formatted = f"{info.name} {OutputTimestamp(info.ts)} {archive_id_hex}" logger.info(f"Analyzing archive {formatted} ({i + 1}/{num_archives})") @@ -3187,9 +3228,18 @@ def check_order(info): logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.") record_result(archive_id, found_before) continue - cdata = self.repository.get(archive_id) try: + cdata = self.repository.get(archive_id) _, data = self.repo_objs.parse(archive_id, cdata, ro_type=ROBJ_ARCHIVE_META) + except Repository.StoreReadError as err: + # unreadable, not corrupt: we did not see the metadata, so we can not tell + # whether this archive is fine - and must not "repair" it away, refs #3509. + # no result is recorded for it, so a later check verifies it again. + logger.error(f"Archive metadata block {archive_id_hex} could not be read: {err}") + self.error_found = True + if self.repair: + raise + continue except IntegrityErrorBase as integrity_error: logger.error(f"Archive metadata block {archive_id_hex} is corrupted: {integrity_error}") self.error_found = True @@ -3207,15 +3257,24 @@ def check_order(info): items_buffer = ChunkBuffer(self.key) items_buffer.write_chunk = add_callback complete = False - for item in robust_iterator(archive): - if sig_int and not self.repair: - # without --repair the archive is only read, so the check also stops within it. - break - if "chunks" in item: - verify_file_chunks(info.name, item) - items_buffer.add(item) - else: - complete = True + try: + for item in robust_iterator(archive): + if sig_int and not self.repair: + # without --repair the archive is only read, so the check also stops within it. + break + if "chunks" in item: + verify_file_chunks(info.name, item) + items_buffer.add(item) + else: + complete = True + except Repository.StoreReadError as err: + # part of the item metadata stream could not be read (see above): rewriting the + # archive from what we did read would drop the rest of it, refs #3509. + logger.error(f"Archive {info.name} {archive_id_hex} could not be read fully: {err}") + self.error_found = True + if self.repair: + raise + continue items_buffer.flush(flush=True) if self.repair: archive.item_ptrs = archive_put_items( diff --git a/src/borg/archiver/check_cmd.py b/src/borg/archiver/check_cmd.py index f297e7aeed..2af1c9d363 100644 --- a/src/borg/archiver/check_cmd.py +++ b/src/borg/archiver/check_cmd.py @@ -300,6 +300,24 @@ def build_parser_check(self, subparsers, common_parser, mid_common_parser): packs the repair wrote, makes the chunk index match them and stores it; that always runs to completion, also after a Ctrl-C. + Unreadable repository objects + +++++++++++++++++++++++++++++ + + A repository object that cannot be read at all (an I/O error from a failing disk, a + flaky network filesystem, ...) is different from a corrupt one: Borg never saw its + content, so it cannot tell whether the object is fine. Such an object is reported and + the check continues, so one run lists everything that is affected; the check then + fails. The object is not remembered as corrupt, so a later check verifies it again. + + Because the data may well be readable again once the underlying problem is fixed, + ``--repair`` refuses to repair while there are unreadable objects, and + ``--verify-data`` leaves chunks it could not read in place instead of deleting them. + Fix the storage hardware, filesystem or network first, then run the check again. + + Only ``borg check`` goes on like this. Other commands stop with the read error rather + than work with a partially readable repository - e.g. ``borg prune`` would otherwise + see an archive whose metadata it could not read as undated and thus as the oldest one. + About repair mode +++++++++++++++++ diff --git a/src/borg/cache.py b/src/borg/cache.py index fafee9c110..2885cb924a 100644 --- a/src/borg/cache.py +++ b/src/borg/cache.py @@ -907,8 +907,9 @@ def build_chunkindex_from_repo( # read completely, and never write to the repo. # write_immediately: store the index (all of it, deleting all other fragments) before returning it. The # callers hold an exclusive lock (borg compact, borg repo-compress, borg check --repair). - # a corrupt fragment raises CorruptChunkIndexFragment: a corrupt index aborts the command. Only with - # write_immediately (the index is rewritten anyway), the index is rebuilt from the packs instead. + # a corrupt fragment raises CorruptChunkIndexFragment, an unreadable one Repository.StoreReadError: an + # unusable index aborts the command. Only with write_immediately (the index is rewritten anyway), the + # index is rebuilt from the packs instead. # validate: a repo object validator or None, passed to PackReader.iter_headers. With a validator, # the rebuild skips the objects that fail it; without one, a corrupt object header raises CorruptPack. # on_drop: a callable or None, passed to PackReader.iter_headers, called once per byte range the @@ -949,7 +950,7 @@ def build_chunkindex_from_repo( break chunks = ChunkIndex() # we'll merge all fragments into this complete = True - corrupt_fragment = None + unusable_fragment = None # the error of a fragment that is corrupt or could not be read msgid = "cache.merge_chunkindex_fragments" show_percent = progress_wanted() if show_percent: @@ -961,8 +962,9 @@ def build_chunkindex_from_repo( pi.show() try: chunks_to_merge = read_chunkindex_from_repo(repository, hash) - except CorruptChunkIndexFragment as err: - corrupt_fragment = err + except (CorruptChunkIndexFragment, Repository.StoreReadError) as err: + # a corrupt fragment, or one that could not be read (I/O error, refs #3509). + unusable_fragment = err break if chunks_to_merge is None: logger.debug(f"chunk index fragment {hash} vanished, restarting the merge...") @@ -977,13 +979,17 @@ def build_chunkindex_from_repo( pi.show(current=len(hashes)) # finish at 100% finally: pi.finish() - if corrupt_fragment is not None: - # retrying would re-read the same corrupt fragment. abort, unless the index gets rewritten + if unusable_fragment is not None: + # retrying would re-read the same unusable fragment. abort, unless the index gets rewritten # anyway: then rebuild the whole index from the packs. chunks.clear() if not write_immediately: - raise corrupt_fragment - logger.warning(f"{corrupt_fragment.args[0]} is corrupt, rebuilding the chunk index from the packs.") + raise unusable_fragment + if isinstance(unusable_fragment, CorruptChunkIndexFragment): + problem = f"{unusable_fragment.args[0]} is corrupt" + else: + problem = f"chunk index fragment {hash} could not be read: {unusable_fragment}" + logger.warning(f"{problem}, rebuilding the chunk index from the packs.") break if complete: if len(hashes) > 1 and write_immediately: diff --git a/src/borg/manifest.py b/src/borg/manifest.py index 6a9697375c..8d0f36badd 100644 --- a/src/borg/manifest.py +++ b/src/borg/manifest.py @@ -195,12 +195,29 @@ def ids(self, *, deleted=False): info = ItemInfo(*info) # RPC does not give us a NamedTuple yield hex_to_bin(info.name) - def _get_archive_meta(self, id: bytes) -> tuple[dict, ArchiveItem | None]: + def _get_archive_meta(self, id: bytes, *, tolerate_read_errors: bool = False) -> tuple[dict, ArchiveItem | None]: # get all metadata directly from the ArchiveItem in the repo, see _parse_archive_meta. from .repository import Repository try: cdata = self.repository.get(id) + except Repository.StoreReadError: + # the archive metadata could not be read at all (I/O error, refs #3509). only borg check + # opts into a placeholder here, so it can go on and check the other archives. everybody + # else must not act on a repository it can not fully read: the placeholder's 1970 + # timestamp would e.g. make prune treat the archive as the oldest one. + if not tolerate_read_errors: + raise + metadata = dict( + id=id, + name="archive-metadata-could-not-be-read", + time="1970-01-01T00:00:00.000000", + exists=False, # we have the pointer, but we could not read the archive item + username="", + hostname="", + tags=(), + ) + return metadata, None except Repository.ObjectNotFound: cdata = None return self._parse_archive_meta(id, cdata) @@ -253,15 +270,28 @@ def _parse_archive_meta(self, id: bytes, cdata) -> tuple[dict, ArchiveItem | Non ) return metadata, archive_item - def _infos(self, *, deleted=False): + def _infos(self, *, deleted=False, tolerate_read_errors=False): # yield (info dict, ArchiveItem) of all archives, reading their metadata objects in batches # (see Repository.gather_many). - ids = list(self.ids(deleted=deleted)) - for id, cdata in zip(ids, self.repository.gather_many(ids, raise_missing=False)): - yield self._parse_archive_meta(id, cdata) + from .repository import Repository - def _info_tuples(self, *, deleted=False): - for info, archive_item in self._infos(deleted=deleted): + ids = list(self.ids(deleted=deleted)) + done = 0 # the number of ids yielded so far + try: + for id, cdata in zip(ids, self.repository.gather_many(ids, raise_missing=False)): + yield self._parse_archive_meta(id, cdata) + done += 1 + except Repository.StoreReadError: + # an archive metadata object could not be read (I/O error, refs #3509), see _get_archive_meta. + if not tolerate_read_errors: + raise + # a batch fails as a whole, so read the remaining objects one by one: only the unreadable + # ones become placeholders. + for id in ids[done:]: + yield self._get_archive_meta(id, tolerate_read_errors=True) + + def _info_tuples(self, *, deleted=False, tolerate_read_errors=False): + for info, archive_item in self._infos(deleted=deleted, tolerate_read_errors=tolerate_read_errors): yield ArchiveInfo( name=info["name"], id=info["id"], @@ -272,8 +302,8 @@ def _info_tuples(self, *, deleted=False): metadata=archive_item, ) - def _matching_info_tuples(self, match_patterns, match_end, *, deleted=False): - archive_infos = list(self._info_tuples(deleted=deleted)) + def _matching_info_tuples(self, match_patterns, match_end, *, deleted=False, tolerate_read_errors=False): + archive_infos = list(self._info_tuples(deleted=deleted, tolerate_read_errors=tolerate_read_errors)) if match_patterns: assert isinstance(match_patterns, list), f"match_pattern is a {type(match_patterns)}" for match in match_patterns: @@ -433,6 +463,7 @@ def list( oldest=None, newest=None, deleted=False, + tolerate_read_errors=False, ): """ Return list of ArchiveInfo instances according to the parameters. @@ -454,7 +485,9 @@ def list( if isinstance(sort_by, (str, bytes)): raise TypeError("sort_by must be a sequence of str") - archive_infos = self._matching_info_tuples(match, match_end, deleted=deleted) + archive_infos = self._matching_info_tuples( + match, match_end, deleted=deleted, tolerate_read_errors=tolerate_read_errors + ) if any([oldest, newest, older, newer]): archive_infos = filter_archives_by_date( diff --git a/src/borg/repository.py b/src/borg/repository.py index 4ceb987c70..0d62bc1f80 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -434,7 +434,13 @@ def read(self, offset, size): # in-memory pack: return a memoryview into pack_contents. store: range-read bytes. if self.pack_contents is not None: return memoryview(self.pack_contents)[offset : offset + size] - return self.store.load(self.key, offset=offset, size=size) + try: + return self.store.load(self.key, offset=offset, size=size) + except OSError as exc: + # the pack exists, but reading it failed (bad disk, flaky network fs, ...). that is + # neither "object missing" nor "content corrupt", so it gets its own error class: + # callers must not react to it by dropping data, refs #3509. + raise Repository.StoreReadError(f"pack {bin_to_hex(self.pack_id)}", exc) from exc def size(self): """Return the pack size in bytes (a store metadata lookup, unless the pack is in memory or pack_size is set).""" @@ -442,7 +448,10 @@ def size(self): return len(self.pack_contents) if self.pack_size is not None: return self.pack_size - return self.store.info(self.key).size + try: + return self.store.info(self.key).size + except OSError as exc: + raise Repository.StoreReadError(f"pack {bin_to_hex(self.pack_id)}", exc) from exc @staticmethod def _parse_header(hdr_data, offset, pack_size): @@ -1003,6 +1012,18 @@ class DefaultsMissing(Error): exit_mcode = 34 + class RepairUnsafe(Error): + """Not repairing: {} repository object(s) could not be read. Fix the underlying problem, then check again.""" + + exit_mcode = 36 + + class StoreReadError(Error): + """Error reading {} from the repository: {}. Check the storage hardware / filesystem.""" + + exit_mcode = 35 + + # the underlying I/O error (bad disk, flaky network fs, ...) is kept as __cause__, refs #3509. + # Whole packs kept in memory for reads; the least recently used is evicted first. # Memory use is this count times the pack size. PACK_READER_CACHE_SIZE = 3 @@ -1311,6 +1332,9 @@ def _load_config(self): # the store exists, but has no repository config: a repository that lost its config, something # that never was a borg 2 repository, or the leftover of an interrupted repo-create (see create()). raise _ConfigMissing() from None + except OSError as exc: + # a config we could not read is not a missing one: do not let it look like "no repository", #3509. + raise self.StoreReadError("config/config", exc) from exc except UnicodeDecodeError: raise self.InvalidRepository(str(self._location)) from None config = configparser.ConfigParser(interpolation=None) @@ -1671,12 +1695,19 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=None, validat assert repo_only is not None or not repair def verify(namespace, name): + """Return True if the object is intact, False if it is corrupt, None if it could not be read.""" # name is the store hash of the object's content, so it is intact iff store.hash() matches. key = f"{namespace}/{name}" try: ok = self.store.hash(key, algorithm=STORE_HASH_NAME) == name except StoreObjectNotFound: return True # vanished since store.list(); not an error + except OSError as exc: + # reading failed (bad disk, flaky network fs, ...), so we do not know whether the + # object is intact. report it and go on: one unreadable object must not abort the + # whole check, and the result must not be recorded as "corrupt", refs #3509. + logger.error(f"Store object {key} could not be read: {exc}") + return None if not ok: logger.error(f"Store object {key} is corrupted: content does not match its name (store hash).") return ok @@ -1686,6 +1717,10 @@ def store_list(namespace): return list(self.store.list(namespace)) except StoreObjectNotFound: return [] # namespace does not exist + except OSError as exc: + # without a listing we do not know what to check, so this one is fatal - but it + # ends the check with a clear message instead of a traceback, refs #3509. + raise self.StoreReadError(f"the {namespace}/ listing", exc) from exc partial = bool(max_duration) assert not (repair and partial) @@ -1710,6 +1745,9 @@ def store_list(namespace): salvaged = salvage_lossy = 0 # packs salvaged, and those of them that may have lost chunks reread_intact = 0 # packs that failed the pack check and read intact at the salvage chunks = None # the index read from its fragments; None if it was not read + # objects that could not be read at all (I/O errors, refs #3509) - counted apart from + # corruption: "unreadable" is a different diagnosis and is often transient/fixable. + read_errors = 0 missing_pack_ids = [] # packs referenced by the index but absent from packs/ (refs #9898) stale_entries = 0 # number of index entries removed for missing_pack_ids index_repaired = False @@ -1732,7 +1770,10 @@ def store_list(namespace): self._lock_refresh() index_pi.show(increase=1) index_files += 1 - if not verify("index", info.name): + result = verify("index", info.name) + if result is None: + read_errors += 1 + elif not result: index_errors += 1 if index_infos: index_pi.show(current=len(index_infos)) # finish at 100% @@ -1773,6 +1814,9 @@ def store_list(namespace): ) index_errors += 1 chunks = None + except self.StoreReadError: + # a fragment could not be read (I/O error, refs #3509), the index check above reports it. + chunks = None if chunks is None: logger.warning( "Cannot cross-check packs against the chunk index: the index could not be loaded " @@ -1847,9 +1891,14 @@ def recorded_ts(info): continue pack_files += 1 ok = verify("packs", info.name) - if not ok: - pack_errors += 1 - tracker.record(pack_id, ok) + if ok is None: + # unreadable: we did not learn anything about this pack, so do not record a + # result for it - a later check re-verifies it (unrecorded packs come first). + read_errors += 1 + else: + if not ok: + pack_errors += 1 + tracker.record(pack_id, ok) now = time.monotonic() # a checkpoint rewrites the whole table (41 bytes per pack), so keep the interval long. if now > t_last_checkpoint + 30 * 60: @@ -1866,7 +1915,9 @@ def recorded_ts(info): pack_pi.finish() # salvage before the index rebuild below, which rebuilds only if no pack is left corrupt. try: - if repair and not sig_int and pack_files + pack_skipped == len(pack_infos): + # not with read errors: repairing around unreadable packs could throw away data that reads fine + # again once the underlying problem is fixed, refs #3509 (the repair is refused below). + if repair and not sig_int and read_errors == 0 and pack_files + pack_skipped == len(pack_infos): # a pack skipped by max_age has an intact record, so it is not salvaged anyway and # counts as scanned here; only an interrupted or time-boxed loop must block a salvage. salvaged, salvage_lossy, reread_intact = self._salvage_corrupt_packs( @@ -1879,12 +1930,15 @@ def recorded_ts(info): # this run: sig_int breaks the loop early, so "no corrupt pack" must be paired with "all packs # scanned" (pack_files == len(pack_infos)) to not rebuild from unverified packs. max_age was # set to 0 after the index cross-check for exactly this path, so no pack was skipped. + # read_errors == 0 for the same reason: an unreadable pack was not verified either, and + # rebuilding the index from packs we could not read would drop their chunks, refs #3509. if ( repair and repo_only and index_errors and invalid_pack_names == 0 and not tracker.corrupt_ids() + and read_errors == 0 and not sig_int and pack_files == len(pack_infos) ): @@ -1927,6 +1981,13 @@ def note_drop(): if pack_skipped: summary += f" Reused {pack_skipped} recent pack check result(s)." logger.info(summary) + if read_errors: + logger.error( + f"{read_errors} store object(s) could not be read, so they could not be checked. " + "This usually means a hardware, filesystem or network problem - see the " + '"Data integrity" section of the docs. The repository was not modified; fix the ' + "underlying problem, then run the check again." + ) if missing_pack_ids: # one id per line (the list can be long). logger.error(f"{len(missing_pack_ids)} pack(s) referenced by the index are missing:") @@ -1951,14 +2012,17 @@ def note_drop(): logger.error(f"Found {len(corrupt_ids)} corrupt pack(s):") for pack_id in corrupt_ids: logger.error(f"Corrupt pack: {bin_to_hex(pack_id)}") - # fail if this run found errors, or any pack is recorded corrupt. - problems = objs_errors != 0 or bool(corrupt_ids) + # fail if this run found errors or could not read some objects, or any pack is recorded corrupt. + problems = objs_errors != 0 or read_errors != 0 or bool(corrupt_ids) # On Ctrl-C the check stopped early, so the summary only covers the packs seen so far. done, so_far = ("Interrupted", " so far") if sig_int else ("Finished", "") if not problems: logger.info(f"{done} {mode} repository check, no problems found{so_far}.") elif not repair: logger.error(f"{done} {mode} repository check, errors found{so_far}.") + elif read_errors: + # repair mode, but unreadable objects stop us below - so report like a read-only check. + logger.error(f"{done} {mode} repository check, errors found{so_far}.") elif invalid_pack_names or corrupt_ids: # corrupt packs not salvaged, e.g. packs in which no object authenticates. if repo_only: @@ -2017,6 +2081,10 @@ def note_drop(): if repair: if index_errors and not index_repaired and not index_deferred: return False + if read_errors: + # we do not know what is in those objects, so repairing around them could throw away + # data that reads fine again once the underlying problem is fixed, refs #3509. + raise self.RepairUnsafe(read_errors) if reread_intact: return False return not (repo_only and (invalid_pack_names or corrupt_ids or missing_pack_ids or drops or salvage_lossy)) @@ -2117,7 +2185,11 @@ def _cached_pack_reader(self, pack_id): reader = self._pack_cache.get(pack_id) if reader is None: key = "packs/" + bin_to_hex(pack_id) - reader = PackReader(pack_id=pack_id, pack_contents=self.store.load(key)) + try: + pack_contents = self.store.load(key) + except OSError as exc: + raise self.StoreReadError(f"pack {bin_to_hex(pack_id)}", exc) from exc + reader = PackReader(pack_id=pack_id, pack_contents=pack_contents) self._pack_cache[pack_id] = reader return reader @@ -2142,6 +2214,7 @@ def get_many(self, ids, read_data=True, raise_missing=True): raise self.PackNotFound(id_, entry.pack_id, str(self._location)) from None yield None else: + # the pack is in memory here, so read() only slices it and can not raise OSError. yield reader.read(entry.obj_offset, entry.obj_size) def gather_many(self, ids, raise_missing=True): @@ -2189,6 +2262,10 @@ def _gather_batch(self, ids, raise_missing): if sources: try: data = self.store.gather([source for _, source in sources], namespace="packs") + except OSError as exc: + # reading failed (bad disk, flaky network fs, ...): neither missing nor corrupt, refs #3509. + packs = {pack for _, (pack, _, _) in sources} + raise self.StoreReadError(f"{len(sources)} objects from {len(packs)} pack(s)", exc) from exc except (StoreObjectNotFound, ReadRangeError): # a missing or truncated pack: read the objects one by one, so the ones in intact packs are # still returned and the others get get()'s error handling. @@ -2880,10 +2957,15 @@ def store_list(self, name, *, deleted=False): return list(self.store.list(name, deleted=deleted)) except StoreObjectNotFound: return [] + except OSError as exc: + raise self.StoreReadError(f"the {name}/ listing", exc) from exc def store_load(self, name, *, size=None, offset=0): self._lock_refresh() - return self.store.load(name, size=size, offset=offset) + try: + return self.store.load(name, size=size, offset=offset) + except OSError as exc: + raise self.StoreReadError(name, exc) from exc def store_store(self, name, value): self._lock_refresh() diff --git a/src/borg/testsuite/archiver/check_cmd_test.py b/src/borg/testsuite/archiver/check_cmd_test.py index c2d6b50147..f0683baed2 100644 --- a/src/borg/testsuite/archiver/check_cmd_test.py +++ b/src/borg/testsuite/archiver/check_cmd_test.py @@ -1,3 +1,4 @@ +import errno import gc import os from pathlib import Path @@ -2029,9 +2030,7 @@ def test_verify_data_reports_a_missing_pack(archivers, request, monkeypatch): assert len(logged) == 2 assert logged[0] == f"pack {bin_to_hex(gone)} is missing, {len(gone_chunks)} chunks are lost." # the lost chunks count as verified and as errors, as if each had been read and failed. - assert logged[-1].endswith( - f"verified {len(repository.chunks)} chunks with {len(gone_chunks)} integrity errors." - ) + assert logged[-1].endswith(f"verified {len(repository.chunks)} chunks with {len(gone_chunks)} error(s).") # each pack was loaded once, the missing one included, and the scan continued past it. packs = {entry.pack_id for _, entry in repository.chunks.iteritems()} assert sorted(loaded) == sorted("packs/" + bin_to_hex(pack_id) for pack_id in packs) @@ -2730,3 +2729,366 @@ def test_check_max_duration_used_up_by_the_repository_check(archiver, monkeypatc assert "Skipping the archive check: the repository check used up --max-duration." in output assert "Starting archive consistency check" not in output assert archive_records(archiver) == {} + + +def make_store_reads_fail(monkeypatch, should_fail, *, after=0): + """Make matching posixfs reads fail with an OSError, like failing storage does. + + should_fail(name, offset, size) decides per read; offset/size are None for the operations that + do not take a range (hash, info, list). The first matching reads still succeed, which + models storage that starts failing (or fails only sometimes) rather than being dead from the + start. + + Patches the backend rather than using file permissions, so this also works when the tests run + as root and does not depend on the platform's permission semantics. Returns a dict whose + "failing" entry switches the failures off again (monkeypatch.undo() must not be used here, it + would also revert the autouse clean_env fixture). + """ + from borgstore.backends.posixfs import PosixFS + + state = {"failing": True, "hits": 0} + orig = {name: getattr(PosixFS, name) for name in ("hash", "load", "info", "list")} + + def check(name, offset=None, size=None): + if not (state["failing"] and should_fail(name, offset, size)): + return + state["hits"] += 1 + if state["hits"] > after: + raise OSError(errno.EIO, "Input/output error", name) + + def failing_hash(self, name, algorithm="sha256"): + check(name) + return orig["hash"](self, name, algorithm=algorithm) + + def failing_load(self, name, *, size=None, offset=0): + check(name, offset, size) + return orig["load"](self, name, size=size, offset=offset) + + def failing_info(self, name): + check(name) + return orig["info"](self, name) + + def failing_list(self, name): + check(name) + return orig["list"](self, name) + + monkeypatch.setattr(PosixFS, "hash", failing_hash) + monkeypatch.setattr(PosixFS, "load", failing_load) + monkeypatch.setattr(PosixFS, "info", failing_info) + monkeypatch.setattr(PosixFS, "list", failing_list) + return state + + +def object_name(name): + """Return the object's own name, without the nesting levels borgstore puts in front of it.""" + # the backend gets the name including those levels, e.g. packs/d0/d0a6... for packs/d0a6... + return name.rsplit("/", 1)[-1] + + +def make_pack_unreadable(monkeypatch, pack_name): + """Make every read of the pack packs/ fail.""" + return make_store_reads_fail(monkeypatch, lambda name, offset, size: object_name(name) == pack_name) + + +def make_namespace_listing_fail(monkeypatch, namespace): + """Make listing the given store namespace fail.""" + return make_store_reads_fail(monkeypatch, lambda name, offset, size: name.split("/")[0] == namespace) + + +def some_pack_name(archiver): + """Return the name of one of the repository's pack files.""" + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + return sorted(info.name for info in repository.store_list("packs"))[0] + + +def pack_name_of(archiver, chunk_id): + """Return the name of the pack file holding chunk_id.""" + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + return bin_to_hex(repository.chunks[chunk_id].pack_id) + + +def file_content_chunk_id(archiver, archive_name="archive1"): + """Return the id of a file content chunk of the given archive.""" + archive, repository = open_archive(archiver.repository_path, archive_name) + with repository: + for item in archive.iter_items(): + if item.path.endswith(src_file): + return item.chunks[-1].id + raise AssertionError(f"{src_file} not found in {archive_name}") + + +def test_check_unreadable_pack(archivers, request, monkeypatch): + # an I/O error while reading a pack must not crash the check with a traceback: it is reported, + # the check goes on and fails at the end, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + cmd(archiver, "check", exit_code=0) + pack_name = some_pack_name(archiver) + make_pack_unreadable(monkeypatch, pack_name) + + output = cmd(archiver, "check", "-v", "--repository-only", exit_code=1) + assert f"Store object packs/{pack_name} could not be read" in output + assert "Input/output error" in output + # the check did not stop at the unreadable pack ... + assert "Finished checking packs." in output + assert "store object(s) could not be read" in output + # ... and it did not claim the pack is corrupt (we never saw its content). + assert "is corrupted" not in output + assert "Corrupt pack" not in output + + +def test_check_unreadable_pack_not_recorded(archivers, request, monkeypatch): + # a pack we could not read gets no result recorded, so a later check verifies it again instead + # of remembering it as corrupt, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + pack_name = some_pack_name(archiver) + state = make_pack_unreadable(monkeypatch, pack_name) + cmd(archiver, "check", "--repository-only", exit_code=1) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + tracker = PackTracker.load(repository) + assert tracker.get(hex_to_bin(pack_name)) is None + assert tracker.corrupt_ids() == [] + # once the pack reads fine again, the check passes without any manual cleanup. + state["failing"] = False + cmd(archiver, "check", exit_code=0) + + +def test_check_repair_refuses_unreadable_pack(archivers, request, monkeypatch): + # --repair must not repair around an unreadable pack: its chunks may well be readable again + # once the underlying problem is fixed, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + pack_name = some_pack_name(archiver) + make_pack_unreadable(monkeypatch, pack_name) + with pytest.raises(Repository.RepairUnsafe): # local (not forked): the Error propagates + cmd(archiver, "check", "--repair") + + +def test_check_repair_does_not_salvage_while_a_pack_is_unreadable(archivers, request, monkeypatch): + # --repair salvages corrupt packs, but not while another pack could not be read: the repair is + # refused then, so it must not have changed anything before, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + chunk_id = file_content_chunk_id(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + corrupt_pack_name = bin_to_hex(repository.chunks[chunk_id].pack_id) + corrupt_chunk_on_disk(repository, chunk_id) + unreadable_pack_name = next( + info.name for info in repository.store_list("packs") if info.name != corrupt_pack_name + ) + state = make_pack_unreadable(monkeypatch, unreadable_pack_name) + with pytest.raises(Repository.RepairUnsafe): # local (not forked): the Error propagates + cmd(archiver, "check", "--repair", "--repository-only") + state["failing"] = False + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + assert corrupt_pack_name in {info.name for info in repository.store_list("packs")} # not salvaged + + +def test_check_verify_data_unreadable_pack_keeps_chunks(archivers, request, monkeypatch): + # --verify-data deletes chunks whose content is defect, but must keep chunks it could not read + # at all, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + # target the pack holding file content, so --verify-data is what reads it. + pack_name = pack_name_of(archiver, file_content_chunk_id(archiver)) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + chunks_before = sorted(chunk_id for chunk_id, _ in repository.chunks.iteritems()) + state = make_pack_unreadable(monkeypatch, pack_name) + + output = cmd(archiver, "check", "--archives-only", "--verify-data", exit_code=1) + assert "could not be read and were left untouched" in output + + state["failing"] = False + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + chunks_after = sorted(chunk_id for chunk_id, _ in repository.chunks.iteritems()) + assert chunks_after == chunks_before # nothing was thrown away + + +def test_check_unreadable_archive_metadata_pack(archivers, request, monkeypatch): + # the pack holding an archive's metadata is unreadable: listing the archives must not die, the + # affected archive is reported and the other archives still get checked, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + archive_id = archive.id + pack_name = pack_name_of(archiver, archive_id) + make_pack_unreadable(monkeypatch, pack_name) + + output = cmd(archiver, "check", "--archives-only", exit_code=1) + assert f"Archive metadata block {bin_to_hex(archive_id)} could not be read" in output + assert "Input/output error" in output + assert "Archive consistency check complete, problems found." in output + + +def test_unreadable_archive_metadata_pack_does_not_fake_an_archive(archivers, request, monkeypatch): + # outside of borg check, an unreadable archive metadata object must not turn into a placeholder + # entry: acting on a repository we can not fully read (e.g. prune, which would see the + # placeholder's 1970 timestamp) is how transient I/O errors become data loss, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + archive_id = archive.id + make_pack_unreadable(monkeypatch, pack_name_of(archiver, archive_id)) + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "repo-list") + + +def test_check_unreadable_packs_listing(archivers, request, monkeypatch): + # without a listing we do not know what to check, so this one is fatal - but it must end the + # check with a clear message instead of a traceback, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_namespace_listing_fail(monkeypatch, "packs") + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check", "--repository-only") + + +def test_check_unreadable_index_object(archivers, request, monkeypatch): + # an unreadable index object is reported and the check goes on to the packs. the missing-pack + # cross-check needs the index too, so it is skipped rather than reporting bogus results, #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + index_name = sorted(info.name for info in repository.store_list("index"))[0] + make_store_reads_fail(monkeypatch, lambda name, offset, size: object_name(name) == index_name) + + output = cmd(archiver, "check", "-v", "--repository-only", exit_code=1) + assert f"Store object index/{index_name} could not be read" in output + assert "skipping missing-pack detection" in output + assert "Finished checking packs." in output # the packs were checked anyway + assert "store object(s) could not be read" in output + + +def test_check_repair_unreadable_pack_aborts_index_rebuild(archivers, request, monkeypatch): + # --repair rebuilds the chunk index from the packs' object headers. an unreadable pack stops + # that with a clear error: an index rebuilt without its objects would drop them, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_pack_unreadable(monkeypatch, some_pack_name(archiver)) + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check", "--archives-only", "--repair") + + +def test_verify_data_repair_keeps_a_chunk_that_fails_to_re_read(archivers, request, monkeypatch): + # --verify-data --repair deletes a chunk only after its content failed to verify twice. if the + # second read fails outright, we did not see the content again, so the chunk must stay: this is + # exactly how a flaky disk would otherwise talk borg into throwing data away, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + chunk_id = file_content_chunk_id(archiver) + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + entry = repository.chunks[chunk_id] + corrupt_chunk_on_disk(repository, chunk_id) + pack_name = bin_to_hex(entry.pack_id) + # fail reads of exactly this object's range (a bad spot in the pack that the first read still got + # through): the first read loads the whole pack and returns the corrupted content, so the chunk + # lands in the defect list, and the re-read that would confirm it (a range read) fails. matching on + # the object's full range leaves the pack's header scan (a short read at the same offset) working, + # so rebuilding the chunk index from the packs still succeeds. + make_store_reads_fail( + monkeypatch, + lambda name, offset, size: ( + object_name(name) == pack_name and offset == entry.obj_offset and size == entry.obj_size + ), + ) + + output = cmd(archiver, "check", "--repair", "--verify-data", "--archives-only", exit_code=0) + assert "not deleted, could not be re-read" in output + with KeyedRepository(archiver.repository_location, exclusive=True) as repository: + assert chunk_id in repository.chunks # the chunk is still there + + +def test_find_lost_archives_skips_unreadable_chunk(archivers, request, monkeypatch): + # the --find-lost-archives scan only looks for archive metadata, so skipping an unreadable + # chunk can at worst miss a lost archive - it never drops data, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_pack_unreadable(monkeypatch, pack_name_of(archiver, file_content_chunk_id(archiver))) + + output = cmd(archiver, "check", "--archives-only", "--find-lost-archives", exit_code=1) + assert "Skipping unreadable chunk" in output + + +def test_unreadable_archives_listing_is_not_an_empty_repository(archivers, request, monkeypatch): + # a namespace listing that fails must not look like "the namespace is empty" - that would make + # e.g. repo-list report a repository with no archives at all, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + make_namespace_listing_fail(monkeypatch, "archives") + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "repo-list") + + +def test_check_repair_stops_at_unreadable_archive_metadata(archivers, request, monkeypatch): + # a bad spot inside an otherwise readable pack: the chunk index still rebuilds from the pack's + # headers, so --repair gets as far as the archive itself - and must stop there rather than + # rewrite the archive around metadata it never read, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + archive, repository = open_archive(archiver.repository_path, "archive1") + with repository: + archive_id = archive.id + entry = repository.chunks[archive_id] + pack_name = bin_to_hex(entry.pack_id) + # fail reads of the archive metadata object only; the short header reads at the same offset + # (and every other object in the pack) keep working. + state = make_store_reads_fail( + monkeypatch, + lambda name, offset, size: ( + object_name(name) == pack_name and offset == entry.obj_offset and size == entry.obj_size + ), + ) + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check", "--archives-only", "--repair") + + # the archive is untouched: once it reads again, it is still there and still checks out. + state["failing"] = False + assert "archive1" in cmd(archiver, "repo-list") + cmd(archiver, "check", exit_code=0) + + +def test_unreadable_repo_config_is_not_a_missing_repository(archivers, request, monkeypatch): + # config/config is what makes the store a repository. if it can not be read, that must not look like + # "no repository here" or like a lost config - both are conclusions we did not earn, refs #3509. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("only works locally, patches objects") + check_cmd_setup(archiver) + state = make_store_reads_fail(monkeypatch, lambda name, offset, size: name == "config/config") + with pytest.raises(Repository.StoreReadError): # local (not forked): the Error propagates + cmd(archiver, "check") + + state["failing"] = False + assert "archive1" in cmd(archiver, "repo-list") # the repository was fine all along + cmd(archiver, "check", exit_code=0) diff --git a/src/borg/testsuite/archives_test.py b/src/borg/testsuite/archives_test.py index 563aea50a3..7f4c2dc70f 100644 --- a/src/borg/testsuite/archives_test.py +++ b/src/borg/testsuite/archives_test.py @@ -54,13 +54,15 @@ def _archiveinfo(name, id_, ts=TS, *, username="", hostname="", tags=()): def _stub_matching_info_tuples(infos): ar, _, _ = _archives() - ar._matching_info_tuples = Mock(side_effect=lambda match_patterns, match_end, deleted=False: list(infos)) + ar._matching_info_tuples = Mock( + side_effect=lambda match_patterns, match_end, deleted=False, tolerate_read_errors=False: list(infos) + ) return ar def _stub_info_tuples(infos): ar, _, _ = _archives() - ar._info_tuples = Mock(side_effect=lambda deleted=False: iter(infos)) + ar._info_tuples = Mock(side_effect=lambda deleted=False, tolerate_read_errors=False: iter(infos)) return ar @@ -385,7 +387,9 @@ def test_archive_infos_carry_metadata(): # the ArchiveInfos made by list(), get() and get_by_id() carry the parsed ArchiveItem. archive_item = object() # stands in for the ArchiveItem ar, repo, _ = _archives() - ar._infos = Mock(side_effect=lambda deleted=False: iter([(_archive_meta("a", _id(1)), archive_item)])) + ar._infos = Mock( + side_effect=lambda deleted=False, tolerate_read_errors=False: iter([(_archive_meta("a", _id(1)), archive_item)]) + ) ar._get_archive_meta = Mock(side_effect=lambda id_: (_archive_meta("a", _id(1)), archive_item)) repo.store_list.return_value = [_iteminfo(_id(1))] assert [info.metadata for info in ar.list()] == [archive_item] @@ -501,9 +505,9 @@ def test_list_date_filter(): def test_list_deleted_passes_flag(): ar, _, _ = _archives() - ar._info_tuples = Mock(side_effect=lambda deleted=False: iter([])) + ar._info_tuples = Mock(side_effect=lambda deleted=False, tolerate_read_errors=False: iter([])) ar.list(deleted=True) - ar._info_tuples.assert_called_once_with(deleted=True) + ar._info_tuples.assert_called_once_with(deleted=True, tolerate_read_errors=False) def test_list_match_name(): @@ -600,9 +604,10 @@ def test_get_one_multiple_matches_raises(): def test_get_one_deleted_passes_flag(): i1 = _archiveinfo("a", _id(1)) ar, _, _ = _archives() - ar._info_tuples = Mock(side_effect=lambda deleted=False: iter([i1])) + ar._info_tuples = Mock(side_effect=lambda deleted=False, tolerate_read_errors=False: iter([i1])) ar.get_one(["a"], deleted=True) - ar._info_tuples.assert_called_once_with(deleted=True) + # get_one never opts into tolerance: it must not return a placeholder for an unreadable archive. + ar._info_tuples.assert_called_once_with(deleted=True, tolerate_read_errors=False) def test_list_considering_raises_if_name_set():