diff --git a/src/blob.rs b/src/blob.rs index ceae79180..f7780d2af 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -32,11 +32,11 @@ impl<'a> BlobObject<'a> { /// /// # Errors /// - /// [BlobErrorKind::CreateFailure] is used when the file could not + /// [BlobError::CreateFailure] is used when the file could not /// be created. You can expect [BlobError.cause] to contain an /// underlying error. /// - /// [BlobErrorKind::WriteFailure] is used when the file could not + /// [BlobError::WriteFailure] is used when the file could not /// be written to. You can expect [BlobError.cause] to contain an /// underlying error. pub fn create( @@ -48,7 +48,12 @@ impl<'a> BlobObject<'a> { let (stem, ext) = BlobObject::sanitise_name(suggested_name.as_ref()); let (name, mut file) = BlobObject::create_new_file(&blobdir, &stem, &ext)?; file.write_all(data) - .map_err(|err| BlobError::new_write_failure(blobdir, &name, err))?; + .map_err(|err| BlobError::WriteFailure { + blobdir: blobdir.to_path_buf(), + blobname: name.clone(), + cause: err, + backtrace: failure::Backtrace::new(), + })?; let blob = BlobObject { blobdir, name: format!("$BLOBDIR/{}", name), @@ -71,18 +76,25 @@ impl<'a> BlobObject<'a> { Ok(file) => return Ok((name, file)), Err(err) => { if attempt == max_attempt { - return Err(BlobError::new_create_failure(dir, &name, err)); + return Err(BlobError::CreateFailure { + blobdir: dir.to_path_buf(), + blobname: name, + cause: err, + backtrace: failure::Backtrace::new(), + }); } else { name = format!("{}-{}{}", stem, rand::random::(), ext); } } } } - Err(BlobError::new_create_failure( - dir, - &name, - format_err!("Unreachable code - supposedly"), - )) + // This is supposed to be unreachable, but the compiler doesn't know. + Err(BlobError::CreateFailure { + blobdir: dir.to_path_buf(), + blobname: name, + cause: std::io::Error::new(std::io::ErrorKind::Other, "supposedly unreachable"), + backtrace: failure::Backtrace::new(), + }) } /// Creates a new blob object with unique name by copying an existing file. @@ -95,24 +107,35 @@ impl<'a> BlobObject<'a> { /// # Errors /// /// In addition to the errors in [BlobObject::create] the - /// [BlobErrorKind::CopyFailure] is used when the data can not be + /// [BlobError::CopyFailure] is used when the data can not be /// copied. pub fn create_and_copy( context: &'a Context, src: impl AsRef, ) -> std::result::Result, BlobError> { - let mut src_file = fs::File::open(src.as_ref()).map_err(|err| { - BlobError::new_copy_failure(context.get_blobdir(), "", src.as_ref(), err) + let mut src_file = fs::File::open(src.as_ref()).map_err(|err| BlobError::CopyFailure { + blobdir: context.get_blobdir().to_path_buf(), + blobname: String::from(""), + src: src.as_ref().to_path_buf(), + cause: err, + backtrace: failure::Backtrace::new(), })?; let (stem, ext) = BlobObject::sanitise_name(&src.as_ref().to_string_lossy()); let (name, mut dst_file) = BlobObject::create_new_file(context.get_blobdir(), &stem, &ext)?; + let name_for_err = name.clone(); std::io::copy(&mut src_file, &mut dst_file).map_err(|err| { { // Attempt to remove the failed file, swallow errors resulting from that. - let path = context.get_blobdir().join(&name); + let path = context.get_blobdir().join(&name_for_err); fs::remove_file(path).ok(); } - BlobError::new_copy_failure(context.get_blobdir(), &name, src.as_ref(), err) + BlobError::CopyFailure { + blobdir: context.get_blobdir().to_path_buf(), + blobname: name_for_err, + src: src.as_ref().to_path_buf(), + cause: err, + backtrace: failure::Backtrace::new(), + } })?; let blob = BlobObject { blobdir: context.get_blobdir(), @@ -156,10 +179,10 @@ impl<'a> BlobObject<'a> { /// /// # Errors /// - /// [BlobErrorKind::WrongBlobdir] is used if the path is not in + /// [BlobError::WrongBlobdir] is used if the path is not in /// the blob directory. /// - /// [BlobErrorKind::WrongName] is used if the file name does not + /// [BlobError::WrongName] is used if the file name does not /// remain identical after sanitisation. pub fn from_path( context: &Context, @@ -168,13 +191,21 @@ impl<'a> BlobObject<'a> { let rel_path = path .as_ref() .strip_prefix(context.get_blobdir()) - .map_err(|_| BlobError::new_wrong_blobdir(context.get_blobdir(), path.as_ref()))?; + .map_err(|_| BlobError::WrongBlobdir { + blobdir: context.get_blobdir().to_path_buf(), + src: path.as_ref().to_path_buf(), + backtrace: failure::Backtrace::new(), + })?; if !BlobObject::is_acceptible_blob_name(&rel_path) { - return Err(BlobError::new_wrong_name(path.as_ref())); + return Err(BlobError::WrongName { + blobname: path.as_ref().to_path_buf(), + backtrace: failure::Backtrace::new(), + }); } - let name = rel_path - .to_str() - .ok_or_else(|| BlobError::new_wrong_name(path.as_ref()))?; + let name = rel_path.to_str().ok_or_else(|| BlobError::WrongName { + blobname: path.as_ref().to_path_buf(), + backtrace: failure::Backtrace::new(), + })?; BlobObject::from_name(context, name.to_string()) } @@ -187,7 +218,7 @@ impl<'a> BlobObject<'a> { /// /// # Errors /// - /// [BlobErrorKind::WrongName] is used if the name is not a valid + /// [BlobError::WrongName] is used if the name is not a valid /// blobname, i.e. if [BlobObject::sanitise_name] does modify the /// provided name. pub fn from_name( @@ -199,7 +230,10 @@ impl<'a> BlobObject<'a> { false => name, }; if !BlobObject::is_acceptible_blob_name(&name) { - return Err(BlobError::new_wrong_name(name)); + return Err(BlobError::WrongName { + blobname: PathBuf::from(name), + backtrace: failure::Backtrace::new(), + }); } Ok(BlobObject { blobdir: context.get_blobdir(), @@ -324,171 +358,50 @@ impl<'a> fmt::Display for BlobObject<'a> { } /// Errors for the [BlobObject]. -/// -/// To keep the return type small and thus the happy path fast this -/// stores everything on the heap. -#[derive(Debug)] -pub struct BlobError { - inner: Box, -} - -#[derive(Debug)] -struct BlobErrorInner { - kind: BlobErrorKind, - data: BlobErrorData, - backtrace: failure::Backtrace, -} - -/// Error kind for [BlobError]. -/// -/// Each error kind has associated data in the [BlobErrorData]. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BlobErrorKind { - /// Failed to create the blob. - CreateFailure, - /// Failed to write data to blob. - WriteFailure, - /// Failed to copy data to blob. - CopyFailure, - /// Blob is not in the blobdir. - WrongBlobdir, - /// Blob has a bad name. - /// - /// E.g. the name is not sanitised correctly or contains a - /// sub-directory. - WrongName, -} - -/// Associated data for each [BlobError] error kind. -/// -/// This is not stored directly on the [BlobErrorKind] so that the -/// kind can stay trivially Copy and Eq. It is however possible to -/// create a [BlobError] with mismatching [BlobErrorKind] and -/// [BlobErrorData], don't do that. -/// -/// Any blobname stored here is the bare name, without the `$BLOBDIR` -/// prefix. All data is owned so that errors do not need to be tied -/// to any lifetimes. -#[derive(Debug)] -enum BlobErrorData { +#[derive(Fail, Debug)] +pub enum BlobError { CreateFailure { blobdir: PathBuf, blobname: String, - cause: failure::Error, + #[cause] + cause: std::io::Error, + backtrace: failure::Backtrace, }, WriteFailure { blobdir: PathBuf, blobname: String, - cause: failure::Error, + #[cause] + cause: std::io::Error, + backtrace: failure::Backtrace, }, CopyFailure { blobdir: PathBuf, blobname: String, src: PathBuf, - cause: failure::Error, + #[cause] + cause: std::io::Error, + backtrace: failure::Backtrace, }, WrongBlobdir { blobdir: PathBuf, src: PathBuf, + backtrace: failure::Backtrace, }, WrongName { blobname: PathBuf, + backtrace: failure::Backtrace, }, } -impl BlobError { - pub fn kind(&self) -> BlobErrorKind { - self.inner.kind - } - - fn new_create_failure( - blobdir: impl Into, - blobname: impl Into, - cause: impl Into, - ) -> BlobError { - BlobError { - inner: Box::new(BlobErrorInner { - kind: BlobErrorKind::CreateFailure, - data: BlobErrorData::CreateFailure { - blobdir: blobdir.into(), - blobname: blobname.into(), - cause: cause.into(), - }, - backtrace: failure::Backtrace::new(), - }), - } - } - - fn new_write_failure( - blobdir: impl Into, - blobname: impl Into, - cause: impl Into, - ) -> BlobError { - BlobError { - inner: Box::new(BlobErrorInner { - kind: BlobErrorKind::WriteFailure, - data: BlobErrorData::WriteFailure { - blobdir: blobdir.into(), - blobname: blobname.into(), - cause: cause.into(), - }, - backtrace: failure::Backtrace::new(), - }), - } - } - - fn new_copy_failure( - blobdir: impl Into, - blobname: impl Into, - src: impl Into, - cause: impl Into, - ) -> BlobError { - BlobError { - inner: Box::new(BlobErrorInner { - kind: BlobErrorKind::CopyFailure, - data: BlobErrorData::CopyFailure { - blobdir: blobdir.into(), - blobname: blobname.into(), - src: src.into(), - cause: cause.into(), - }, - backtrace: failure::Backtrace::new(), - }), - } - } - - fn new_wrong_blobdir(blobdir: impl Into, src: impl Into) -> BlobError { - BlobError { - inner: Box::new(BlobErrorInner { - kind: BlobErrorKind::WrongBlobdir, - data: BlobErrorData::WrongBlobdir { - blobdir: blobdir.into(), - src: src.into(), - }, - backtrace: failure::Backtrace::new(), - }), - } - } - - fn new_wrong_name(blobname: impl Into) -> BlobError { - BlobError { - inner: Box::new(BlobErrorInner { - kind: BlobErrorKind::WrongName, - data: BlobErrorData::WrongName { - blobname: blobname.into(), - }, - backtrace: failure::Backtrace::new(), - }), - } - } -} - +// Implementing Display is done by hand because the failure +// #[fail(display = "...")] syntax does not allow using +// `blobdir.display()`. impl fmt::Display for BlobError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Match on the data rather than kind, they are equivalent for // identifying purposes but contain the actual data we need. - match &self.inner.data { - BlobErrorData::CreateFailure { + match &self { + BlobError::CreateFailure { blobdir, blobname, .. } => write!( f, @@ -496,7 +409,7 @@ impl fmt::Display for BlobError { blobname, blobdir.display() ), - BlobErrorData::WriteFailure { + BlobError::WriteFailure { blobdir, blobname, .. } => write!( f, @@ -504,7 +417,7 @@ impl fmt::Display for BlobError { blobname, blobdir.display() ), - BlobErrorData::CopyFailure { + BlobError::CopyFailure { blobdir, blobname, src, @@ -516,34 +429,19 @@ impl fmt::Display for BlobError { blobname, blobdir.display(), ), - BlobErrorData::WrongBlobdir { blobdir, src } => write!( + BlobError::WrongBlobdir { blobdir, src, .. } => write!( f, "File path {} is not in blobdir {}", src.display(), blobdir.display(), ), - BlobErrorData::WrongName { blobname } => { + BlobError::WrongName { blobname, .. } => { write!(f, "Blob has a bad name: {}", blobname.display(),) } } } } -impl failure::Fail for BlobError { - fn cause(&self) -> Option<&dyn failure::Fail> { - match &self.inner.data { - BlobErrorData::CreateFailure { cause, .. } - | BlobErrorData::WriteFailure { cause, .. } - | BlobErrorData::CopyFailure { cause, .. } => Some(cause.as_fail()), - _ => None, - } - } - - fn backtrace(&self) -> Option<&failure::Backtrace> { - Some(&self.inner.backtrace) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/chat.rs b/src/chat.rs index 30b10dcff..06580134a 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use itertools::Itertools; use num_traits::FromPrimitive; -use crate::blob::{BlobErrorKind, BlobObject}; +use crate::blob::{BlobError, BlobObject}; use crate::chatlist::*; use crate::config::*; use crate::constants::*; @@ -1797,8 +1797,8 @@ pub fn set_chat_profile_image( )); } else { let image_blob = BlobObject::from_path(context, Path::new(new_image.as_ref())).or_else( - |err| match err.kind() { - BlobErrorKind::WrongBlobdir => { + |err| match err { + BlobError::WrongBlobdir { .. } => { BlobObject::create_and_copy(context, Path::new(new_image.as_ref())) } _ => Err(err), diff --git a/src/param.rs b/src/param.rs index 524fcedf6..908ba7923 100644 --- a/src/param.rs +++ b/src/param.rs @@ -321,7 +321,6 @@ mod tests { use std::fs; use std::path::Path; - use crate::blob::BlobErrorKind; use crate::test_utils::*; #[test] @@ -404,7 +403,10 @@ mod tests { // Blob does not exist yet, expect BlobError. let err = p.get_blob(Param::File, &t.ctx, false).unwrap_err(); - assert_eq!(err.kind(), BlobErrorKind::WrongBlobdir); + match err { + BlobError::WrongBlobdir { .. } => (), + _ => panic!("wrong error type/variant: {:?}", err), + } fs::write(fname, b"boo").unwrap(); let blob = p.get_blob(Param::File, &t.ctx, true).unwrap().unwrap();