fix: count unreadable source files during backup - #561
Conversation
Track scan and archival errors instead of silently ignoring them, and expose the count on SnapshotSummary so callers can return restic exit code 3 for incomplete snapshots.
|
Companion CLI PR: rustic-rs/rustic#1908 |
| /// Report a source-file error, count it, and continue the backup. | ||
| fn report_source_error(p: &Progress, errors: &AtomicU64, during: &'static str, err: &RusticError) { | ||
| _ = errors.fetch_add(1, Ordering::Relaxed); | ||
| let item = err.context_value("path").unwrap_or(""); |
There was a problem hiding this comment.
instead of the unwrap and later checking for an empty string, please work with an Option
There was a problem hiding this comment.
Done — Progress::error now takes Option<&str>, and report_source_error passes err.context_value("path") through without converting a missing path to an empty string.
| /// * `during` - What was being attempted, e.g. `"scan"` or `"archival"` | ||
| /// * `message` - Error message | ||
| fn error(&self, item: &str, during: &str, message: &str) { | ||
| if item.is_empty() { |
There was a problem hiding this comment.
Same change here: the trait method now takes Option<&str> and the default logger matches on Some/None instead of checking for an empty string.
| #[cfg(unix)] | ||
| struct RestorePerms<'a>(&'a Path); | ||
|
|
||
| #[cfg(unix)] |
There was a problem hiding this comment.
Why do we need to restore permissions for a file which is anyway located just in a temp dir?
There was a problem hiding this comment.
Agreed, it was unnecessary. unlink only needs write permission on the parent directory, so the RestorePerms drop guard is gone.
| /// | ||
| /// This is not persisted in the snapshot file; it is only set on a snapshot | ||
| /// returned from a just-completed backup. |
There was a problem hiding this comment.
Actually I suggest to also persist this information in case the error count > 0. This is then a quite important information...
There was a problem hiding this comment.
Now serialized when non-zero (skip_serializing_if zero, default on deserialize) so existing snapshots stay unchanged and incomplete backups keep the count. The integration test reloads the snapshot from the repo to check it round-trips.
Pass source paths as Option through Progress::error instead of empty strings. Persist SnapshotSummary.error_count when it is non-zero. Drop the chmod restore in the tempdir test; unlink only needs directory permissions.
2 participants
Summary
Progress::error(scan/archival)SnapshotSummary.error_countfor the just-completed backupWhy
restic returns exit code 3 when some source files cannot be read, and still creates a snapshot of the remaining files. rustic currently ignores those errors and exits 0, so automation cannot detect incomplete backups.
Validation
cargo test --test integration test_backup_unreadable_file_sets_error_count(unix; chmod 000 file)cargo fmt --allA companion rustic CLI PR will use
error_countto exit 3 and emit restic-compatible JSONerror/exit_errormessages.