diff --git a/src/error.rs b/src/error.rs index dc93e5c..7880b5a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,47 +1,96 @@ //! Error and Result definitions -use std::fmt::{Display, Formatter}; -use std::io::Error; +use std::{error, fmt, io}; /// The error type of this crate #[derive(Debug)] -pub enum VfsError { - /// A generic IO error - IoError(std::io::Error), +pub struct VfsError { + /// The path this error was encountered in + path: String, + /// The kind of error + kind: VfsErrorKind, + /// An optional human-readable string describing the context for this error + /// + /// If not provided, a generic context message is used + context: String, + /// The underlying error + cause: Option>, +} - /// The file or directory at the given path could not be found - FileNotFound { - /// The path of the file not found - path: String, - }, +/// The only way to create a VfsError is via a VfsErrorKind +/// +/// This conversion implements certain normalizations +impl From for VfsError { + fn from(kind: VfsErrorKind) -> Self { + // Normalize the error here before we return it + let kind = match kind { + VfsErrorKind::IoError(io) => match io.kind() { + io::ErrorKind::NotFound => VfsErrorKind::FileNotFound, + // TODO: If MSRV changes to 1.53, enable this. Alternatively, + // if it's possible to #[cfg] just this line, try that + // io::ErrorKind::Unsupported => VfsErrorKind::NotSupported, + _ => VfsErrorKind::IoError(io), + }, + // Remaining kinda are passed through as-is + other => other, + }; - /// The given path is invalid, e.g. because contains '.' or '..' - InvalidPath { - /// The invalid path - path: String, - }, + Self { + // TODO (Techno): See if this could be checked at compile-time to make sure the VFS abstraction + // never forgets to add a path. Might need a separate error type for FS impls vs VFS + path: "PATH NOT FILLED BY VFS LAYER".into(), + kind, + context: "An error occured".into(), + cause: None, + } + } +} - /// Generic error variant - Other { - /// The generic error message - message: String, - }, - - /// Generic error context, used for adding context to an error (like a path) - WithContext { - /// The context error message - context: String, - /// The underlying error - cause: Box, - }, +impl From for VfsError { + fn from(err: io::Error) -> Self { + Self::from(VfsErrorKind::IoError(err)) + } +} - /// Functionality not supported by this filesystem - NotSupported, +impl VfsError { + // Path filled by the VFS crate rather than the implementations + pub(crate) fn with_path(mut self, path: impl Into) -> Self { + self.path = path.into(); + self + } + + pub fn with_context(mut self, context: F) -> Self + where + C: fmt::Display + Send + Sync + 'static, + F: FnOnce() -> C, + { + self.context = context().to_string(); + self + } + + pub fn with_cause(mut self, cause: VfsError) -> Self { + self.cause = Some(Box::new(cause)); + self + } + + pub fn kind(&self) -> &VfsErrorKind { + &self.kind + } + + pub fn path(&self) -> &String { + &self.path + } +} + +impl fmt::Display for VfsError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} for '{}': {}", self.context, self.path, self.kind()) + } } -impl std::error::Error for VfsError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - if let Self::WithContext { cause, .. } = self { +impl error::Error for VfsError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + if let Some(cause) = &self.cause { Some(cause) } else { None @@ -49,37 +98,43 @@ impl std::error::Error for VfsError { } } -impl From for VfsError { - fn from(message: String) -> Self { - VfsError::Other { message } - } -} +/// The kinds of errors that can occur +#[derive(Debug)] +pub enum VfsErrorKind { + /// A generic I/O error + /// + /// Certain standard I/O errors are normalized to their VfsErrorKind counterparts + IoError(io::Error), -impl From for VfsError { - fn from(cause: Error) -> Self { - VfsError::IoError(cause) - } + /// The file or directory at the given path could not be found + FileNotFound, + + /// The given path is invalid, e.g. because contains '.' or '..' + InvalidPath, + + /// Generic error variant + Other(String), + + /// Functionality not supported by this filesystem + NotSupported, } -impl std::fmt::Display for VfsError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for VfsErrorKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - VfsError::IoError(cause) => { + VfsErrorKind::IoError(cause) => { write!(f, "IO error: {}", cause) } - VfsError::FileNotFound { path } => { - write!(f, "The file or directory `{}` could not be found", path) + VfsErrorKind::FileNotFound => { + write!(f, "The file or directory could not be found") } - VfsError::InvalidPath { path } => { - write!(f, "The path `{}` is invalid", path) + VfsErrorKind::InvalidPath => { + write!(f, "The path is invalid") } - VfsError::Other { message } => { + VfsErrorKind::Other(message) => { write!(f, "FileSystem error: {}", message) } - VfsError::WithContext { context, cause } => { - write!(f, "{}, cause: {}", context, cause) - } - VfsError::NotSupported => { + VfsErrorKind::NotSupported => { write!(f, "Functionality not supported by this filesystem") } } @@ -88,24 +143,3 @@ impl std::fmt::Display for VfsError { /// The result type of this crate pub type VfsResult = std::result::Result; - -/// Result extension trait to add context information -pub(crate) trait VfsResultExt { - fn with_context(self, f: F) -> VfsResult - where - C: Display + Send + Sync + 'static, - F: FnOnce() -> C; -} - -impl VfsResultExt for VfsResult { - fn with_context(self, context: F) -> VfsResult - where - C: Display + Send + Sync + 'static, - F: FnOnce() -> C, - { - self.map_err(|error| VfsError::WithContext { - context: context().to_string(), - cause: Box::new(error), - }) - } -} diff --git a/src/filesystem.rs b/src/filesystem.rs index ef2071c..0d986b8 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -1,6 +1,7 @@ //! The filesystem trait definitions needed to implement new virtual filesystems -use crate::{SeekAndRead, VfsError, VfsMetadata, VfsPath, VfsResult}; +use crate::error::VfsErrorKind; +use crate::{SeekAndRead, VfsMetadata, VfsPath, VfsResult}; use std::fmt::Debug; use std::io::Write; @@ -35,15 +36,15 @@ pub trait FileSystem: Debug + Sync + Send + 'static { fn remove_dir(&self, path: &str) -> VfsResult<()>; /// Copies the src path to the destination path within the same filesystem (optional) fn copy_file(&self, _src: &str, _dest: &str) -> VfsResult<()> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } /// Moves the src path to the destination path within the same filesystem (optional) fn move_file(&self, _src: &str, _dest: &str) -> VfsResult<()> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } /// Moves the src directory to the destination path within the same filesystem (optional) fn move_dir(&self, _src: &str, _dest: &str) -> VfsResult<()> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } } diff --git a/src/impls/altroot.rs b/src/impls/altroot.rs index ff2e84a..a966272 100644 --- a/src/impls/altroot.rs +++ b/src/impls/altroot.rs @@ -1,6 +1,6 @@ //! A file system with its root in a particular directory of another filesystem -use crate::{FileSystem, SeekAndRead, VfsError, VfsMetadata, VfsPath, VfsResult}; +use crate::{error::VfsErrorKind, FileSystem, SeekAndRead, VfsMetadata, VfsPath, VfsResult}; use std::io::Write; /// Similar to a chroot but done purely by path manipulation @@ -78,7 +78,7 @@ impl FileSystem for AltrootFS { fn copy_file(&self, src: &str, dest: &str) -> VfsResult<()> { if dest.is_empty() { - return Err(VfsError::NotSupported); + return Err(VfsErrorKind::NotSupported.into()); } self.path(src)?.copy_file(&self.path(dest)?) } diff --git a/src/impls/embedded.rs b/src/impls/embedded.rs index 2cdeec5..fadedd4 100644 --- a/src/impls/embedded.rs +++ b/src/impls/embedded.rs @@ -6,7 +6,8 @@ use std::marker::PhantomData; use rust_embed::RustEmbed; -use crate::{FileSystem, SeekAndRead, VfsError, VfsFileType, VfsMetadata, VfsResult}; +use crate::error::VfsErrorKind; +use crate::{FileSystem, SeekAndRead, VfsFileType, VfsMetadata, VfsResult}; type EmbeddedPath = Cow<'static, str>; @@ -88,35 +89,29 @@ where } else { if self.files.contains_key(normalized_path) { // Actually a file - return Err(VfsError::Other { - message: format!("{} is not a directory", path), - }); + return Err(VfsErrorKind::Other("Not a directory".into()).into()); } - Err(VfsError::FileNotFound { - path: path.to_string(), - }) + Err(VfsErrorKind::FileNotFound.into()) } } fn create_dir(&self, _path: &str) -> VfsResult<()> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } fn open_file(&self, path: &str) -> VfsResult> { match T::get(path.split_at(1).1) { - None => Err(VfsError::FileNotFound { - path: path.to_string(), - }), + None => Err(VfsErrorKind::FileNotFound.into()), Some(file) => Ok(Box::new(Cursor::new(file.data))), } } fn create_file(&self, _path: &str) -> VfsResult> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } fn append_file(&self, _path: &str) -> VfsResult> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } fn metadata(&self, path: &str) -> VfsResult { @@ -133,9 +128,7 @@ where len: 0, }); } - Err(VfsError::FileNotFound { - path: path.to_string(), - }) + Err(VfsErrorKind::FileNotFound.into()) } fn exists(&self, path: &str) -> VfsResult { @@ -154,11 +147,11 @@ where } fn remove_file(&self, _path: &str) -> VfsResult<()> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } fn remove_dir(&self, _path: &str) -> VfsResult<()> { - Err(VfsError::NotSupported) + Err(VfsErrorKind::NotSupported.into()) } } @@ -175,7 +168,7 @@ mod tests { use std::collections::HashSet; use std::io::Read; - use crate::{FileSystem, VfsError, VfsFileType, VfsPath}; + use crate::{FileSystem, VfsFileType, VfsPath}; use super::*; @@ -217,40 +210,50 @@ mod tests { #[test] fn read_dir_no_directory_err() { let fs = get_test_fs(); - assert!(match fs.read_dir("/c/f") { - Err(VfsError::FileNotFound { .. }) => true, - _ => false, - }); - assert!(match fs.read_dir("/a.txt.") { - Err(VfsError::FileNotFound { .. }) => true, - _ => false, - }); - assert!(match fs.read_dir("/abc/def/ghi") { - Err(VfsError::FileNotFound { .. }) => true, + assert!(match fs.read_dir("/c/f").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::FileNotFound => true, _ => false, }); + assert!( + match fs.read_dir("/a.txt.").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::FileNotFound => true, + _ => false, + } + ); + assert!( + match fs.read_dir("/abc/def/ghi").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::FileNotFound => true, + _ => false, + } + ); } #[test] fn read_dir_on_file_err() { let fs = get_test_fs(); - assert!(match fs.read_dir("/a.txt") { - Err(VfsError::Other { message }) => &message == "/a.txt is not a directory", - _ => false, - }); - assert!(match fs.read_dir("/a/d.txt") { - Err(VfsError::Other { message }) => &message == "/a/d.txt is not a directory", - _ => false, - }); + assert!( + match fs.read_dir("/a.txt").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::Other(message) => message == "Not a directory", + _ => false, + } + ); + assert!( + match fs.read_dir("/a/d.txt").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::Other(message) => message == "Not a directory", + _ => false, + } + ); } #[test] fn create_dir_not_supported() { let fs = get_test_fs(); - assert!(match fs.create_dir("/abc") { - Err(VfsError::NotSupported) => true, - _ => false, - }) + assert!( + match fs.create_dir("/abc").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::NotSupported => true, + _ => false, + } + ) } #[test] @@ -278,16 +281,27 @@ mod tests { #[test] fn open_file_not_found() { let fs = get_test_fs(); + // FIXME: These tests have been weakened since the FS implementations aren't intended to + // provide paths for errors. Maybe this could be handled better assert!(match fs.open_file("/") { - Err(VfsError::FileNotFound { path }) => path.as_str() == "/", + Err(err) => match err.kind() { + VfsErrorKind::FileNotFound => true, + _ => false, + }, _ => false, }); assert!(match fs.open_file("/abc.txt") { - Err(VfsError::FileNotFound { path }) => path.as_str() == "/abc.txt", + Err(err) => match err.kind() { + VfsErrorKind::FileNotFound => true, + _ => false, + }, _ => false, }); assert!(match fs.open_file("/c/f.txt") { - Err(VfsError::FileNotFound { path }) => path.as_str() == "/c/f.txt", + Err(err) => match err.kind() { + VfsErrorKind::FileNotFound => true, + _ => false, + }, _ => false, }); } @@ -295,19 +309,23 @@ mod tests { #[test] fn create_file_not_supported() { let fs = get_test_fs(); - assert!(match fs.create_file("/abc.txt") { - Err(VfsError::NotSupported) => true, - _ => false, - }); + assert!( + match fs.create_file("/abc.txt").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::NotSupported => true, + _ => false, + } + ); } #[test] fn append_file_not_supported() { let fs = get_test_fs(); - assert!(match fs.append_file("/abc.txt") { - Err(VfsError::NotSupported) => true, - _ => false, - }); + assert!( + match fs.append_file("/abc.txt").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::NotSupported => true, + _ => false, + } + ); } #[test] @@ -343,7 +361,10 @@ mod tests { fn metadata_not_found() { let fs = get_test_fs(); assert!(match fs.metadata("/abc.txt") { - Err(VfsError::FileNotFound { path }) => path.as_str() == "/abc.txt", + Err(err) => match err.kind() { + VfsErrorKind::FileNotFound => true, + _ => false, + }, _ => false, }); } @@ -368,19 +389,23 @@ mod tests { #[test] fn remove_file_not_supported() { let fs = get_test_fs(); - assert!(match fs.remove_file("/abc.txt") { - Err(VfsError::NotSupported) => true, - _ => false, - }); + assert!( + match fs.remove_file("/abc.txt").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::NotSupported => true, + _ => false, + } + ); } #[test] fn remove_dir_not_supported() { let fs = get_test_fs(); - assert!(match fs.remove_dir("/abc.txt") { - Err(VfsError::NotSupported) => true, - _ => false, - }); + assert!( + match fs.remove_dir("/abc.txt").map(|_| ()).unwrap_err().kind() { + VfsErrorKind::NotSupported => true, + _ => false, + } + ); } #[test] diff --git a/src/impls/memory.rs b/src/impls/memory.rs index d971a44..5e8481e 100644 --- a/src/impls/memory.rs +++ b/src/impls/memory.rs @@ -1,8 +1,9 @@ //! An ephemeral in-memory file system, intended mainly for unit tests +use crate::error::VfsErrorKind; +use crate::VfsResult; use crate::{FileSystem, VfsFileType}; use crate::{SeekAndRead, VfsMetadata}; -use crate::{VfsError, VfsResult}; use core::cmp; use std::collections::HashMap; use std::fmt; @@ -39,9 +40,7 @@ impl MemoryFS { return Ok(()); } } - Err(VfsError::Other { - message: format!("Parent path of {} does not exist", path), - }) + Err(VfsErrorKind::Other("Parent path does not exist".into()).into()) } } @@ -143,9 +142,7 @@ impl FileSystem for MemoryFS { }) .collect(); if !found_directory { - return Err(VfsError::FileNotFound { - path: path.to_string(), - }); + return Err(VfsErrorKind::FileNotFound.into()); } Ok(Box::new(entries.into_iter())) } @@ -164,12 +161,7 @@ impl FileSystem for MemoryFS { fn open_file(&self, path: &str) -> VfsResult> { let handle = self.handle.read().unwrap(); - let file = handle - .files - .get(path) - .ok_or_else(|| VfsError::FileNotFound { - path: path.to_string(), - })?; + let file = handle.files.get(path).ok_or(VfsErrorKind::FileNotFound)?; ensure_file(file)?; Ok(Box::new(ReadableFile { content: file.content.clone(), @@ -197,12 +189,7 @@ impl FileSystem for MemoryFS { fn append_file(&self, path: &str) -> VfsResult> { let handle = self.handle.write().unwrap(); - let file = handle - .files - .get(path) - .ok_or_else(|| VfsError::FileNotFound { - path: path.to_string(), - })?; + let file = handle.files.get(path).ok_or(VfsErrorKind::FileNotFound)?; let mut content = Cursor::new(file.content.as_ref().clone()); content.seek(SeekFrom::End(0))?; let writer = WritableFile { @@ -216,9 +203,7 @@ impl FileSystem for MemoryFS { fn metadata(&self, path: &str) -> VfsResult { let guard = self.handle.read().unwrap(); let files = &guard.files; - let file = files.get(path).ok_or_else(|| VfsError::FileNotFound { - path: path.to_string(), - })?; + let file = files.get(path).ok_or(VfsErrorKind::FileNotFound)?; Ok(VfsMetadata { file_type: file.file_type, len: file.content.len() as u64, @@ -234,25 +219,19 @@ impl FileSystem for MemoryFS { handle .files .remove(path) - .ok_or_else(|| VfsError::FileNotFound { - path: path.to_string(), - })?; + .ok_or(VfsErrorKind::FileNotFound)?; Ok(()) } fn remove_dir(&self, path: &str) -> VfsResult<()> { if self.read_dir(path)?.next().is_some() { - return Err(VfsError::Other { - message: "Directory to remove is not empty".to_string(), - }); + return Err(VfsErrorKind::Other("Directory to remove is not empty".into()).into()); } let mut handle = self.handle.write().unwrap(); handle .files .remove(path) - .ok_or_else(|| VfsError::FileNotFound { - path: path.to_string(), - })?; + .ok_or(VfsErrorKind::FileNotFound)?; Ok(()) } } @@ -342,7 +321,10 @@ mod tests { let root = VfsPath::new(MemoryFS::new()); let path = root.join("foo").unwrap(); let result = path.remove_dir(); - assert_eq!(format!("{}", result.unwrap_err()), "Could not remove directory '/foo', cause: The file or directory `/foo` could not be found"); + assert_eq!( + format!("{}", result.unwrap_err()), + "Could not remove directory for '/foo': The file or directory could not be found" + ); } #[test] @@ -353,7 +335,10 @@ mod tests { match result { Ok(_) => panic!("Error expected"), Err(err) => { - assert_eq!(format!("{}", err), "Could not read directory '/foo', cause: The file or directory `/foo` could not be found"); + assert_eq!( + format!("{}", err), + "Could not read directory for '/foo': The file or directory could not be found" + ); } } } @@ -373,9 +358,7 @@ mod tests { fn ensure_file(file: &MemoryFile) -> VfsResult<()> { if file.file_type != VfsFileType::File { - return Err(VfsError::Other { - message: "Not a file".to_string(), - }); + return Err(VfsErrorKind::Other("Not a file".into()).into()); } Ok(()) } diff --git a/src/impls/overlay.rs b/src/impls/overlay.rs index ad54962..a758796 100644 --- a/src/impls/overlay.rs +++ b/src/impls/overlay.rs @@ -1,7 +1,7 @@ //! An overlay file system combining two filesystems, an upper layer with read/write access and a lower layer with only read access -use crate::error::VfsResultExt; -use crate::{FileSystem, SeekAndRead, VfsError, VfsMetadata, VfsPath, VfsResult}; +use crate::error::VfsErrorKind; +use crate::{FileSystem, SeekAndRead, VfsMetadata, VfsPath, VfsResult}; use std::collections::HashSet; use std::io::Write; @@ -36,9 +36,7 @@ impl OverlayFS { return Ok(self.layers[0].clone()); } if self.whiteout_path(path)?.exists()? { - return Err(VfsError::FileNotFound { - path: path.to_string(), - }); + return Err(VfsErrorKind::FileNotFound.into()); } for layer in &self.layers { let layer_path = layer.join(&path[1..])?; @@ -48,9 +46,7 @@ impl OverlayFS { } let read_path = self.write_layer().join(&path[1..])?; if !read_path.exists()? { - return Err(VfsError::FileNotFound { - path: path.to_string(), - }); + return Err(VfsErrorKind::FileNotFound.into()); } Ok(read_path) } @@ -79,9 +75,7 @@ impl OverlayFS { return Ok(()); } } - Err(VfsError::Other { - message: format!("Parent path of '{}' does not exist", path), - }) + Err(VfsErrorKind::Other("Parent path does not exist".into()).into()) } } @@ -89,9 +83,7 @@ impl FileSystem for OverlayFS { fn read_dir(&self, path: &str) -> VfsResult>> { let actual_path = if !path.is_empty() { &path[1..] } else { path }; if !self.read_path(path)?.exists()? { - return Err(VfsError::FileNotFound { - path: path.to_string(), - }); + return Err(VfsErrorKind::FileNotFound.into()); } let mut entries = HashSet::::new(); for layer in &self.layers { @@ -155,7 +147,7 @@ impl FileSystem for OverlayFS { fn exists(&self, path: &str) -> VfsResult { if self .whiteout_path(path) - .with_context(|| "whiteout_path")? + .map_err(|err| err.with_context(|| "whiteout_path"))? .exists()? { return Ok(false); diff --git a/src/impls/physical.rs b/src/impls/physical.rs index dc516aa..3d8d2f5 100644 --- a/src/impls/physical.rs +++ b/src/impls/physical.rs @@ -1,8 +1,9 @@ //! A "physical" file system implementation using the underlying OS file system +use crate::error::VfsErrorKind; +use crate::VfsResult; use crate::{FileSystem, VfsMetadata}; use crate::{SeekAndRead, VfsFileType}; -use crate::{VfsError, VfsResult}; use std::fs::{File, OpenOptions}; use std::io::Write; use std::path::{Path, PathBuf}; @@ -97,6 +98,7 @@ impl FileSystem for PhysicalFS { fn move_file(&self, src: &str, dest: &str) -> VfsResult<()> { std::fs::rename(self.get_path(src), self.get_path(dest))?; + Ok(()) } @@ -104,7 +106,7 @@ impl FileSystem for PhysicalFS { let result = std::fs::rename(self.get_path(src), self.get_path(dest)); if result.is_err() { // Error possibly due to different filesystems, return not supported and let the fallback handle it - return Err(VfsError::NotSupported); + return Err(VfsErrorKind::NotSupported.into()); } Ok(()) } diff --git a/src/path.rs b/src/path.rs index cff7f08..a1ac33b 100644 --- a/src/path.rs +++ b/src/path.rs @@ -6,7 +6,7 @@ use std::io::{Read, Seek, Write}; use std::sync::Arc; -use crate::error::VfsResultExt; +use crate::error::VfsErrorKind; use crate::{FileSystem, VfsError, VfsResult}; /// Trait combining Seek and Read, return value for opening files @@ -110,9 +110,7 @@ impl VfsPath { let mut base_path = self.clone(); for component in path.split('/') { if component.is_empty() { - return Err(VfsError::InvalidPath { - path: path.to_string(), - }); + return Err(VfsError::from(VfsErrorKind::InvalidPath).with_path(path)); } if component == "." { continue; @@ -123,9 +121,7 @@ impl VfsPath { } else if let Some(parent) = base_path.parent() { base_path = parent; } else { - return Err(VfsError::InvalidPath { - path: path.to_string(), - }); + return Err(VfsError::from(VfsErrorKind::InvalidPath).with_path(path)); } } else { new_components.push(component); @@ -176,10 +172,10 @@ impl VfsPath { /// ``` pub fn create_dir(&self) -> VfsResult<()> { self.get_parent("create directory")?; - self.fs - .fs - .create_dir(&self.path) - .with_context(|| format!("Could not create directory '{}'", &self.path)) + self.fs.fs.create_dir(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not create directory") + }) } /// Creates the directory at this path, also creating parent directories as necessary @@ -244,7 +240,10 @@ impl VfsPath { self.fs .fs .read_dir(&self.path) - .with_context(|| format!("Could not read directory '{}'", &self.path))? + .map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not read directory") + })? .map(move |path| VfsPath { path: format!("{}/{}", parent, path), fs: fs.clone(), @@ -269,10 +268,10 @@ impl VfsPath { /// ``` pub fn create_file(&self) -> VfsResult> { self.get_parent("create file")?; - self.fs - .fs - .create_file(&self.path) - .with_context(|| format!("Could not create file '{}'", &self.path)) + self.fs.fs.create_file(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not create file") + }) } /// Opens the file at this path for reading @@ -291,10 +290,10 @@ impl VfsPath { /// # Ok::<(), VfsError>(()) /// ``` pub fn open_file(&self) -> VfsResult> { - self.fs - .fs - .open_file(&self.path) - .with_context(|| format!("Could not open file '{}'", &self.path)) + self.fs.fs.open_file(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not open file") + }) } /// Checks whether parent is a directory @@ -302,27 +301,27 @@ impl VfsPath { let parent = self.parent(); match parent { None => { - return Err(format!( - "Could not {} at '{}', not a valid location", - action, &self.path - ) - .into()); + return Err(VfsError::from(VfsErrorKind::Other(format!( + "Could not {}, not a valid location", + action + ))) + .with_path(&self.path)); } Some(directory) => { if !directory.exists()? { - return Err(format!( - "Could not {} at '{}', parent directory does not exist", - action, &self.path - ) - .into()); + return Err(VfsError::from(VfsErrorKind::Other(format!( + "Could not {}, parent directory does not exist", + action + ))) + .with_path(&self.path)); } let metadata = directory.metadata()?; if metadata.file_type != VfsFileType::Directory { - return Err(format!( - "Could not {} at '{}', parent path is not a directory", - action, &self.path - ) - .into()); + return Err(VfsError::from(VfsErrorKind::Other(format!( + "Could not {}, parent path is not a directory", + action + ))) + .with_path(&self.path)); } } } @@ -344,10 +343,10 @@ impl VfsPath { /// # Ok::<(), VfsError>(()) /// ``` pub fn append_file(&self) -> VfsResult> { - self.fs - .fs - .append_file(&self.path) - .with_context(|| format!("Could not open file '{}' for appending", &self.path)) + self.fs.fs.append_file(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not open file for appending") + }) } /// Removes the file at this path @@ -366,10 +365,10 @@ impl VfsPath { /// # Ok::<(), VfsError>(()) /// ``` pub fn remove_file(&self) -> VfsResult<()> { - self.fs - .fs - .remove_file(&self.path) - .with_context(|| format!("Could not remove file '{}'", &self.path)) + self.fs.fs.remove_file(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not remove file") + }) } /// Removes the directory at this path @@ -390,10 +389,10 @@ impl VfsPath { /// # Ok::<(), VfsError>(()) /// ``` pub fn remove_dir(&self) -> VfsResult<()> { - self.fs - .fs - .remove_dir(&self.path) - .with_context(|| format!("Could not remove directory '{}'", &self.path)) + self.fs.fs.remove_dir(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not remove directory") + }) } /// Ensures that the directory at this path is removed, recursively deleting all contents if necessary @@ -447,10 +446,10 @@ impl VfsPath { /// assert_eq!(file.metadata()?.file_type, VfsFileType::File); /// # Ok::<(), VfsError>(()) pub fn metadata(&self) -> VfsResult { - self.fs - .fs - .metadata(&self.path) - .with_context(|| format!("Could not get metadata for '{}'", &self.path)) + self.fs.fs.metadata(&self.path).map_err(|err| { + err.with_path(&self.path) + .with_context(|| "Could not get metadata") + }) } /// Returns `true` if the path exists and is pointing at a regular file, otherwise returns `false`. @@ -629,15 +628,20 @@ impl VfsPath { pub fn read_to_string(&self) -> VfsResult { let metadata = self.metadata()?; if metadata.file_type != VfsFileType::File { - return Err(VfsError::Other { - message: format!("Could not read '{}' because it is a directory", self.path), - }); + return Err( + VfsError::from(VfsErrorKind::Other("Path is a directory".into())) + .with_path(&self.path) + .with_context(|| "Could not read path"), + ); } let mut result = String::with_capacity(metadata.len as usize); self.open_file()? .read_to_string(&mut result) - .map_err(From::from) - .with_context(|| format!("Could not read '{}'", self.path))?; + .map_err(|source| { + VfsError::from(source) + .with_path(&self.path) + .with_context(|| "Could not read path") + })?; Ok(result) } @@ -661,27 +665,40 @@ impl VfsPath { pub fn copy_file(&self, destination: &VfsPath) -> VfsResult<()> { || -> VfsResult<()> { if destination.exists()? { - return Err("Destination exists already".to_string().into()); + return Err(VfsError::from(VfsErrorKind::Other( + "Destination exists already".into(), + )) + .with_path(&self.path)); } if Arc::ptr_eq(&self.fs, &destination.fs) { let result = self.fs.fs.copy_file(&self.path, &destination.path); - if let Err(VfsError::NotSupported) = result { - // continue - } else { - return result; + match result { + Err(err) => match err.kind() { + VfsErrorKind::NotSupported => { + // continue + } + _ => return Err(err), + }, + other => return other, } } let mut src = self.open_file()?; let mut dest = destination.create_file()?; - std::io::copy(&mut src, &mut dest)?; + std::io::copy(&mut src, &mut dest).map_err(|source| { + VfsError::from(source) + .with_path(&self.path) + .with_context(|| "Could not read path") + })?; Ok(()) }() - .with_context(|| { - format!( - "Could not copy '{}' to '{}'", - self.as_str(), - destination.as_str() - ) + .map_err(|err| { + err.with_path(&self.path).with_context(|| { + format!( + "Could not copy '{}' to '{}'", + self.as_str(), + destination.as_str() + ) + }) })?; Ok(()) } @@ -707,28 +724,41 @@ impl VfsPath { pub fn move_file(&self, destination: &VfsPath) -> VfsResult<()> { || -> VfsResult<()> { if destination.exists()? { - return Err("Destination exists already".to_string().into()); + return Err(VfsError::from(VfsErrorKind::Other( + "Destination exists already".into(), + )) + .with_path(&destination.path)); } if Arc::ptr_eq(&self.fs, &destination.fs) { let result = self.fs.fs.move_file(&self.path, &destination.path); - if let Err(VfsError::NotSupported) = result { - // continue - } else { - return result; + match result { + Err(err) => match err.kind() { + VfsErrorKind::NotSupported => { + // continue + } + _ => return Err(err), + }, + other => return other, } } let mut src = self.open_file()?; let mut dest = destination.create_file()?; - std::io::copy(&mut src, &mut dest)?; + std::io::copy(&mut src, &mut dest).map_err(|source| { + VfsError::from(source) + .with_path(&self.path) + .with_context(|| "Could not read path") + })?; self.remove_file()?; Ok(()) }() - .with_context(|| { - format!( - "Could not move '{}' to '{}'", - self.as_str(), - destination.as_str() - ) + .map_err(|err| { + err.with_path(&self.path).with_context(|| { + format!( + "Could not move '{}' to '{}'", + self.as_str(), + destination.as_str() + ) + }) })?; Ok(()) } @@ -756,7 +786,10 @@ impl VfsPath { let mut files_copied = 0u64; || -> VfsResult<()> { if destination.exists()? { - return Err("Destination exists already".to_string().into()); + return Err(VfsError::from(VfsErrorKind::Other( + "Destination exists already".into(), + )) + .with_path(&destination.path)); } destination.create_dir()?; let prefix = self.path.as_str(); @@ -772,12 +805,14 @@ impl VfsPath { } Ok(()) }() - .with_context(|| { - format!( - "Could not copy directory '{}' to '{}'", - self.as_str(), - destination.as_str() - ) + .map_err(|err| { + err.with_path(&self.path).with_context(|| { + format!( + "Could not copy directory '{}' to '{}'", + self.as_str(), + destination.as_str() + ) + }) })?; Ok(files_copied) } @@ -803,14 +838,21 @@ impl VfsPath { pub fn move_dir(&self, destination: &VfsPath) -> VfsResult<()> { || -> VfsResult<()> { if destination.exists()? { - return Err("Destination exists already".to_string().into()); + return Err(VfsError::from(VfsErrorKind::Other( + "Destination exists already".into(), + )) + .with_path(&destination.path)); } if Arc::ptr_eq(&self.fs, &destination.fs) { let result = self.fs.fs.move_dir(&self.path, &destination.path); - if let Err(VfsError::NotSupported) = result { - // continue - } else { - return result; + match result { + Err(err) => match err.kind() { + VfsErrorKind::NotSupported => { + // continue + } + _ => return Err(err), + }, + other => return other, } } destination.create_dir()?; @@ -827,12 +869,14 @@ impl VfsPath { self.remove_dir_all()?; Ok(()) }() - .with_context(|| { - format!( - "Could not move directory '{}' to '{}'", - self.as_str(), - destination.as_str() - ) + .map_err(|err| { + err.with_path(&self.path).with_context(|| { + format!( + "Could not move directory '{}' to '{}'", + self.as_str(), + destination.as_str() + ) + }) })?; Ok(()) } diff --git a/src/test_macros.rs b/src/test_macros.rs index 0128274..b0365bd 100644 --- a/src/test_macros.rs +++ b/src/test_macros.rs @@ -67,7 +67,7 @@ macro_rules! test_vfs { Err(err) => { let error_message = format!("{}", err); assert!( - error_message.starts_with("Could not open file '/test_append.txt' for appending"), + error_message.starts_with("Could not open file for appending"), "Actual message: {}", error_message); } @@ -376,19 +376,24 @@ macro_rules! test_vfs { "/foo" ); + /// Utility function for templating the same error message + fn invalid_path_message(path: &str) -> String { + format!("An error occured for '{}': The path is invalid", path) + } + assert_eq!( root.join("..").unwrap_err().to_string(), - "The path `..` is invalid".to_string(), + invalid_path_message(".."), ".." ); assert_eq!( root.join("../foo").unwrap_err().to_string(), - "The path `../foo` is invalid".to_string(), + invalid_path_message("../foo"), "../foo" ); assert_eq!( root.join("foo/../..").unwrap_err().to_string(), - "The path `foo/../..` is invalid".to_string(), + invalid_path_message("foo/../.."), "foo/../.." ); assert_eq!( @@ -397,18 +402,18 @@ macro_rules! test_vfs { .join("../..") .unwrap_err() .to_string(), - "The path `../..` is invalid".to_string(), + invalid_path_message("../.."), "foo+../.." ); assert_eq!( root.join("/").unwrap_err().to_string(), - "The path `/` is invalid".to_string(), + invalid_path_message("/"), "/" ); assert_eq!( root.join("foo/").unwrap_err().to_string(), - "The path `foo/` is invalid".to_string(), + invalid_path_message("foo/"), "foo/" ); } @@ -506,7 +511,7 @@ macro_rules! test_vfs { .expect_err("walk_dir") .to_string(); assert!( - error_message.starts_with("Could not read directory '/foo'"), + error_message.starts_with("Could not read directory for '/foo'"), "Actual message: {}", error_message ); @@ -528,7 +533,7 @@ macro_rules! test_vfs { .expect_err("walk_dir") .to_string(); assert!( - error_message.starts_with("Could not read directory '/foo'"), + error_message.starts_with("Could not read directory for '/foo'"), "Actual message: {}", error_message ); @@ -538,7 +543,7 @@ macro_rules! test_vfs { } #[test] - fn read_as_string() -> VfsResult<()> { + fn read_to_string() -> VfsResult<()> { let root = create_root(); let path = root.join("foobar.txt")?; path.create_file()?.write_all(b"Hello World")?; @@ -547,7 +552,7 @@ macro_rules! test_vfs { } #[test] - fn read_as_string_missing() -> VfsResult<()> { + fn read_to_string_missing() -> VfsResult<()> { let root = create_root(); let error_message = root.join("foobar.txt")?.read_to_string().expect_err("read_to_string").to_string(); assert!( @@ -559,12 +564,12 @@ macro_rules! test_vfs { } #[test] - fn read_as_string_directory() -> VfsResult<()> { + fn read_to_string_directory() -> VfsResult<()> { let root = create_root(); root.join("foobar.txt")?.create_dir()?; let error_message = root.join("foobar.txt")?.read_to_string().expect_err("read_to_string").to_string(); assert!( - error_message.starts_with("FileSystem error: Could not read '/foobar.txt' because it is a directory"), + error_message.starts_with("Could not read path for '/foobar.txt'"), "Actual message: {}", error_message ); @@ -572,14 +577,15 @@ macro_rules! test_vfs { } #[test] - fn read_as_string_nonutf8() -> VfsResult<()> { + fn read_to_string_nonutf8() -> VfsResult<()> { let root = create_root(); let path = root.join("foobar.txt")?; path.create_file()?.write_all(&vec![0, 159, 146, 150])?; let error_message = path.read_to_string().expect_err("read_to_string").to_string(); assert_eq!( &error_message, - "Could not read '/foobar.txt', cause: IO error: stream did not contain valid UTF-8" ); + "Could not read path for '/foobar.txt': IO error: stream did not contain valid UTF-8" + ); Ok(()) } @@ -1126,19 +1132,25 @@ macro_rules! test_vfs_readonly { "/foo" ); + /// Utility function for templating the same error message + // TODO: Maybe deduplicate this function + fn invalid_path_message(path: &str) -> String { + format!("An error occured for '{}': The path is invalid", path) + } + assert_eq!( root.join("..").unwrap_err().to_string(), - "The path `..` is invalid".to_string(), + invalid_path_message(".."), ".." ); assert_eq!( root.join("../foo").unwrap_err().to_string(), - "The path `../foo` is invalid".to_string(), + invalid_path_message("../foo"), "../foo" ); assert_eq!( root.join("foo/../..").unwrap_err().to_string(), - "The path `foo/../..` is invalid".to_string(), + invalid_path_message("foo/../.."), "foo/../.." ); assert_eq!( @@ -1147,18 +1159,18 @@ macro_rules! test_vfs_readonly { .join("../..") .unwrap_err() .to_string(), - "The path `../..` is invalid".to_string(), + invalid_path_message("../.."), "foo+../.." ); assert_eq!( root.join("/").unwrap_err().to_string(), - "The path `/` is invalid".to_string(), + invalid_path_message("/"), "/" ); assert_eq!( root.join("foo/").unwrap_err().to_string(), - "The path `foo/` is invalid".to_string(), + invalid_path_message("foo/"), "foo/" ); } @@ -1219,7 +1231,7 @@ macro_rules! test_vfs_readonly { .expect_err("walk_dir") .to_string(); assert!( - error_message.starts_with("Could not read directory '/foo'"), + error_message.starts_with("Could not read directory for '/foo'"), "Actual message: {}", error_message ); @@ -1227,7 +1239,7 @@ macro_rules! test_vfs_readonly { } #[test] - fn read_as_string() -> VfsResult<()> { + fn read_to_string() -> VfsResult<()> { let root = create_root(); let path = root.join("a.txt")?; assert_eq!(path.read_to_string()?, "a"); @@ -1235,7 +1247,7 @@ macro_rules! test_vfs_readonly { } #[test] - fn read_as_string_missing() -> VfsResult<()> { + fn read_to_string_missing() -> VfsResult<()> { let root = create_root(); let error_message = root .join("foobar.txt")? @@ -1251,7 +1263,7 @@ macro_rules! test_vfs_readonly { } #[test] - fn read_as_string_directory() -> VfsResult<()> { + fn read_to_string_directory() -> VfsResult<()> { let root = create_root(); let error_message = root .join("a")? @@ -1259,9 +1271,7 @@ macro_rules! test_vfs_readonly { .expect_err("read_to_string") .to_string(); assert!( - error_message.starts_with( - "FileSystem error: Could not read '/a' because it is a directory" - ), + error_message.starts_with("Could not read path for '/a'"), "Actual message: {}", error_message );