Reduced role of ErrorTask to simplify errors

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<ErrorTask>,
     source: Option<Box<Error>>,
 }
 
 impl Error {
     /// Get the task being attempted when the error was returned
-    pub fn task(&self) -> &ErrorTask {
+    pub fn task(&self) -> &Option<ErrorTask> {
         &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<K: Into<ErrorKind>> From<K> for Error {
     fn from(kind: K) -> Self {
         Self {
-            task: ErrorTask::UnknownTask,
+            task: None,
             kind: kind.into(),
             source: None,
         }
@@ -125,7 +95,11 @@ impl<K: Into<ErrorKind>> From<K> 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());

src/lib.rs

@@ -102,12 +102,11 @@ impl FileMode {
 
 fn path_to_cstring(path: impl AsRef<Path>) -> Result<CString> {
     use ErrorKind::InvalidOsStr;
-    use ErrorTask::ToCString;
     let s = path
         .as_ref()
         .to_str()
-        .ok_or_else(|| Error::from(InvalidOsStr).with_task(ToCString))?;
+        .ok_or_else(|| Error::from(InvalidOsStr))?;
-    CString::new(s).map_err(|e| Error::from(e).with_task(ToCString))
+    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 (
+            if let ErrorKind::CouldNotOpen {
-                ErrorTask::OpenFile,
+                path: err_path,
-                ErrorKind::CouldNotOpen {
+                mode: err_mode,
-                    path: err_path,
+            } = e.kind()
-                    mode: err_mode,
-                },
-            ) = (e.task(), 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() {
+            assert_eq!(Some(ErrorCode::ExdrMagic), e.code());
-                ErrorTask::ReadNumAtoms => {
+        } else {
-                    assert_eq!(Some(ErrorCode::ExdrMagic), e.code());
+            panic!("Should not be able to read number of atoms from readme");
-                }
+        }
-                _ => panic!("Wrong Error type"),
-            }
-        };
         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() {
+            assert_eq!(Some(ErrorCode::ExdrMagic), e.code());
-                ErrorTask::Read => {
+        } else {
-                    assert_eq!(Some(ErrorCode::ExdrMagic), e.code());
+            panic!("Should not be able to read number of atoms from readme");
-                }
-                _ => panic!("Wrong Error type"),
-            }
         }
         Ok(())
     }