Skip to content

Commit

Permalink
refactor(error): improve error messages and file handling (#334)
Browse files Browse the repository at this point in the history
- Create parent directory if it does not exist before opening the file
for writing.

Fixes rustic-rs/rustic#1315
Fixes #310

---------

Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
  • Loading branch information
simonsan authored Nov 18, 2024
1 parent 2048483 commit 749879f
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 4 deletions.
46 changes: 44 additions & 2 deletions crates/backend/src/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,21 @@ impl LocalBackend {
}
Ok(())
}

/// Returns the parent path of the given file type and id.
///
/// # Arguments
///
/// * `tpe` - The type of the file.
/// * `id` - The id of the file.
///
/// # Returns
///
/// The parent path of the file or `None` if the file does not have a parent.
fn parent_path(&self, tpe: FileType, id: &Id) -> Option<PathBuf> {
let path = self.path(tpe, id);
path.parent().map(Path::to_path_buf)
}
}

impl ReadBackend for LocalBackend {
Expand Down Expand Up @@ -355,13 +370,14 @@ impl ReadBackend for LocalBackend {
length: u32,
) -> RusticResult<Bytes> {
trace!("reading tpe: {tpe:?}, id: {id}, offset: {offset}, length: {length}");
let mut file = File::open(self.path(tpe, id)).map_err(|err| {
let filename = self.path(tpe, id);
let mut file = File::open(filename.clone()).map_err(|err| {
RusticError::with_source(
ErrorKind::Backend,
"Failed to open the file `{path}`. Please check the file and try again.",
err,
)
.attach_context("path", self.path(tpe, id).to_string_lossy())
.attach_context("path", filename.to_string_lossy())
})?;
_ = file.seek(SeekFrom::Start(offset.into())).map_err(|err| {
RusticError::with_source(
Expand Down Expand Up @@ -459,6 +475,10 @@ impl WriteBackend for LocalBackend {
/// * If the length of the file could not be set.
/// * If the bytes could not be written to the file.
/// * If the OS Metadata could not be synced to disk.
/// * If the file does not have a parent directory.
/// * If the parent directory could not be created.
/// * If the file cannot be opened, due to missing permissions.
/// * If the file cannot be written to, due to lack of space on the disk.
fn write_bytes(
&self,
tpe: FileType,
Expand All @@ -469,6 +489,28 @@ impl WriteBackend for LocalBackend {
trace!("writing tpe: {:?}, id: {}", &tpe, &id);
let filename = self.path(tpe, id);

let Some(parent) = self.parent_path(tpe, id) else {
return Err(
RusticError::new(
ErrorKind::Backend,
"The file `{path}` does not have a parent directory. This may be empty or a root path. Please check the file and try again.",
)
.attach_context("path", filename.display().to_string())
.ask_report()
);
};

// create parent directory if it does not exist
fs::create_dir_all(parent.clone()).map_err(|err| {
RusticError::with_source(
ErrorKind::InputOutput,
"Failed to create directories `{path}`. Does the directory already exist? Please check the file and try again.",
err,
)
.attach_context("path", parent.display().to_string())
.ask_report()
})?;

let mut file = fs::OpenOptions::new()
.create(true)
.truncate(true)
Expand Down
18 changes: 16 additions & 2 deletions crates/core/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,24 @@ pub(crate) fn check_repository<P: ProgressBars, S: Open>(

packs.into_par_iter().for_each(|pack| {
let id = pack.id;
let data = be.read_full(FileType::Pack, &id).unwrap();
let data = match be.read_full(FileType::Pack, &id) {
Ok(data) => data,
Err(err) => {
// FIXME: This needs different handling, now it prints a full display of RusticError
// Instead we should actually collect and return a list of errors on the happy path
// for `Check`, as this is a non-critical operation and we want to show all errors
// to the user.
error!("Error reading data for pack {id} : {err}");
return;
}
};
match check_pack(be, pack, data, &p) {
Ok(()) => {}
Err(err) => error!("Error reading pack {id} : {err}",),
// FIXME: This needs different handling, now it prints a full display of RusticError
// Instead we should actually collect and return a list of errors on the happy path
// for `Check`, as this is a non-critical operation and we want to show all errors
// to the user.
Err(err) => error!("Pack {id} is not valid: {err}",),
}
});
p.finish();
Expand Down

0 comments on commit 749879f

Please sign in to comment.