From 420af8b369f5b214c790ca126c89322fe1a25a9d Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Tue, 10 Nov 2020 18:17:23 +1100 Subject: [PATCH] Reduced role of ErrorTask to simplify errors --- src/errors.rs | 93 +++++++++++++++++++++++++-------------------------- src/lib.rs | 46 +++++++++++-------------- 2 files changed, 66 insertions(+), 73 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index e1dfd04..323a9ca 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -3,51 +3,17 @@ use crate::FileMode; use crate::Frame; use std::path::{Path, PathBuf}; -#[derive(Debug, Clone, Copy, PartialEq)] -/// The task being attempted when an error was returned -pub enum ErrorTask { - /// A file was being opened - OpenFile, - /// The number of atoms was being read from a file - ReadNumAtoms, - /// A frame was being read from a file - Read, - /// A frame was being written to a file - Write, - /// An file was being flushed to disk - Flush, - /// A path was being converted to a CString - ToCString, - /// Placeholder until a task can be provided - UnknownTask, -} - -impl std::fmt::Display for ErrorTask { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use ErrorTask::*; - match &self { - OpenFile => write!(f, "Failed to open file as trajectory"), - ReadNumAtoms => write!(f, "Failed to read atom number from trajectory"), - Read => write!(f, "Failed to read trajectory"), - Write => write!(f, "Failed to write trajectory"), - Flush => write!(f, "Failed to flush trajectory"), - ToCString => write!(f, "Failed to convert to CString"), - UnknownTask => write!(f, "Task failed"), - } - } -} - #[derive(Debug, Clone, PartialEq)] /// Error type for the xdrfile library pub struct Error { kind: ErrorKind, - task: ErrorTask, + task: Option, source: Option>, } impl Error { /// Get the task being attempted when the error was returned - pub fn task(&self) -> &ErrorTask { + pub fn task(&self) -> &Option { &self.task } @@ -82,7 +48,7 @@ impl Error { let kind; let source; - if let ErrorTask::UnknownTask = self.task { + if let None = self.task { kind = self.kind; source = None } else { @@ -90,7 +56,11 @@ impl Error { source = Some(Box::new(self)) }; - Self { kind, task, source } + Self { + kind, + task: Some(task), + source, + } } /// Convert an error code and output value from a C call to a Result @@ -106,7 +76,7 @@ impl Error { } else { Err(Self { kind: ErrorKind::from(code), - task, + task: Some(task), source: None, }) } @@ -116,7 +86,7 @@ impl Error { impl> From for Error { fn from(kind: K) -> Self { Self { - task: ErrorTask::UnknownTask, + task: None, kind: kind.into(), source: None, } @@ -125,7 +95,11 @@ impl> From for Error { impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{task}: {kind}", task = self.task, kind = self.kind) + if let Some(task) = self.task { + write!(f, "{task}: {kind}", task = task, kind = self.kind) + } else { + write!(f, "{}", self.kind) + } } } @@ -143,6 +117,31 @@ impl std::error::Error for Error { } } +#[derive(Debug, Clone, Copy, PartialEq)] +/// The task being attempted if ErrorKind is ambiguous +pub enum ErrorTask { + /// The number of atoms was being read from a file + ReadNumAtoms, + /// A frame was being read from a file + Read, + /// A frame was being written to a file + Write, + /// An file was being flushed to disk + Flush, +} + +impl std::fmt::Display for ErrorTask { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use ErrorTask::*; + match &self { + ReadNumAtoms => write!(f, "Failed to read atom number from trajectory"), + Read => write!(f, "Failed to read trajectory"), + Write => write!(f, "Failed to write trajectory"), + Flush => write!(f, "Failed to flush trajectory"), + } + } +} + #[derive(Debug, Clone, PartialEq)] pub enum ErrorKind { /// An error code from the C API @@ -297,35 +296,35 @@ mod tests { use ErrorKind::ErrorCode as Code; let error = Error { kind: Code(c_abi::xdrfile::exdrENDOFFILE.into()), - task: ErrorTask::Read, + task: Some(ErrorTask::Read), source: None, }; assert!(error.is_eof()); let error = Error { kind: Code(ErrorCode::ExdrEndOfFile), - task: ErrorTask::ReadNumAtoms, + task: Some(ErrorTask::ReadNumAtoms), source: None, }; assert!(error.is_eof()); let error = Error { kind: Code((c_abi::xdrfile::exdrENDOFFILE + 1).into()), - task: ErrorTask::Read, + task: None, source: None, }; assert!(!error.is_eof()); let error = Error { kind: Code(0.into()), - task: ErrorTask::Write, + task: Some(ErrorTask::Write), source: None, }; assert!(!error.is_eof()); let error = Error { kind: Code(255.into()), - task: ErrorTask::Flush, + task: Some(ErrorTask::Flush), source: None, }; assert!(!error.is_eof()); @@ -335,7 +334,7 @@ mod tests { path: PathBuf::from("not/a/file"), mode: FileMode::Read, }, - task: ErrorTask::OpenFile, + task: None, source: None, }; assert!(!error.is_eof()); diff --git a/src/lib.rs b/src/lib.rs index 2db2839..2a4d235 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,12 +102,11 @@ impl FileMode { fn path_to_cstring(path: impl AsRef) -> Result { use ErrorKind::InvalidOsStr; - use ErrorTask::ToCString; let s = path .as_ref() .to_str() - .ok_or_else(|| Error::from(InvalidOsStr).with_task(ToCString))?; - CString::new(s).map_err(|e| Error::from(e).with_task(ToCString)) + .ok_or_else(|| Error::from(InvalidOsStr))?; + CString::new(s).map_err(|e| Error::from(e)) } /// A safe wrapper around the c implementation of an XDRFile @@ -140,7 +139,7 @@ impl XDRFile { }) } else { // Something went wrong. But the C api does not tell us what - Err(Error::from((path, filemode)).with_task(ErrorTask::OpenFile)) + Err(Error::from((path, filemode))) } } } @@ -505,7 +504,7 @@ mod tests { let result = xtc_traj.read(&mut frame); if let Err(e) = result { - assert_eq!(e.task(), &ErrorTask::Read); + assert_eq!(e.task(), &Some(ErrorTask::Read)); assert!(if let ErrorKind::WrongSizeFrame { .. } = e.kind() { true } else { @@ -522,7 +521,11 @@ mod tests { let result_invalid = path_to_cstring(PathBuf::from("invalid/\0path")); if let Err(err) = result_invalid { - assert!(err.task() == &ErrorTask::ToCString); + match err.kind() { + ErrorKind::NullInStr(_) => (), + ErrorKind::InvalidOsStr => (), + _ => panic!("Improper error type for path_to_cstring"), + } } else { panic!("path_to_cstring should return Err if there are null bytes"); } @@ -539,13 +542,10 @@ mod tests { let path = Path::new(&file_name); if let Err(e) = XDRFile::open(file_name, FileMode::Read) { - if let ( - ErrorTask::OpenFile, - ErrorKind::CouldNotOpen { - path: err_path, - mode: err_mode, - }, - ) = (e.task(), e.kind()) + if let ErrorKind::CouldNotOpen { + path: err_path, + mode: err_mode, + } = e.kind() { assert_eq!(path, err_path); assert_eq!(FileMode::Read, *err_mode) @@ -560,13 +560,10 @@ mod tests { let file_name = "README.md"; // not a trajectory let mut trr = TRRTrajectory::open_read(file_name)?; if let Err(e) = trr.get_num_atoms() { - match e.task() { - ErrorTask::ReadNumAtoms => { - assert_eq!(Some(ErrorCode::ExdrMagic), e.code()); - } - _ => panic!("Wrong Error type"), - } - }; + assert_eq!(Some(ErrorCode::ExdrMagic), e.code()); + } else { + panic!("Should not be able to read number of atoms from readme"); + } Ok(()) } @@ -576,12 +573,9 @@ mod tests { let mut frame = Frame::with_capacity(1); let mut trr = TRRTrajectory::open_read(file_name)?; if let Err(e) = trr.read(&mut frame) { - match e.task() { - ErrorTask::Read => { - assert_eq!(Some(ErrorCode::ExdrMagic), e.code()); - } - _ => panic!("Wrong Error type"), - } + assert_eq!(Some(ErrorCode::ExdrMagic), e.code()); + } else { + panic!("Should not be able to read number of atoms from readme"); } Ok(()) }