refactor: Use thiserror to simplify error handling

Author: John Doe

hakoniwa/src/child_process/error.rs

@@ -1,24 +1,21 @@
 pub(crate) type Result<T> = std::result::Result<T, Error>;
 
-#[derive(Debug)]
-pub(crate) struct Error(pub String);
-
-impl std::fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0)
-    }
-}
-
-impl std::error::Error for Error {}
-
-impl From<libseccomp::error::SeccompError> for Error {
-    fn from(e: libseccomp::error::SeccompError) -> Self {
-        Self(e.to_string())
-    }
+#[derive(thiserror::Error, Debug)]
+pub(crate) enum Error {
+    #[error(transparent)]
+    SeccompError(#[from] libseccomp::error::SeccompError),
+    #[error(transparent)]
+    PathAbsError(#[from] path_abs::Error),
+    #[error(transparent)]
+    BincodeError(#[from] BincodeErrorKind),
+    #[error("{0}")]
+    SyscallError(String),
 }
 
-impl From<path_abs::Error> for Error {
-    fn from(e: path_abs::Error) -> Self {
-        Self(e.to_string())
-    }
+#[derive(thiserror::Error, Debug)]
+pub enum BincodeErrorKind {
+    #[error(transparent)]
+    EncodeError(#[from] bincode::error::EncodeError),
+    #[error(transparent)]
+    DecodeError(#[from] bincode::error::DecodeError),
 }

hakoniwa/src/child_process/result.rs

@@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize};
 use std::{os::unix::io::RawFd, time::Duration};
 
 use crate::{
-    child_process::{error::Error, error::Result, syscall},
+    child_process::{error::*, syscall},
     ExecutorResultStatus,
 };
 
@@ -40,7 +40,7 @@ impl ChildProcessResult {
         let config = config::standard();
         let encoded: Vec<u8> = match bincode::serde::encode_to_vec(cpr, config) {
             Ok(val) => val,
-            Err(err) => return Err(Error(err.to_string())),
+            Err(err) => return Err(Error::BincodeError(BincodeErrorKind::EncodeError(err))),
         };
         syscall::write(writer, encoded.as_slice()).map(|_| ())
     }
@@ -53,7 +53,7 @@ impl ChildProcessResult {
         let (decoded, _): (Self, usize) =
             match bincode::serde::decode_from_slice(&encoded[..], config) {
                 Ok(val) => val,
-                Err(err) => return Err(Error(err.to_string())),
+                Err(err) => return Err(Error::BincodeError(BincodeErrorKind::DecodeError(err))),
             };
         Ok(decoded)
     }

hakoniwa/src/child_process/syscall.rs

@@ -47,7 +47,7 @@ macro_rules! tryfn {
         $mod::$fn($($arg),*).map_err(|err| {
             let name = stringify!($fn);
             let args = format!($args_format, $($arg),*);
-            Error(format!("{}({}) => {}", name, args, err))
+            Error::SyscallError(format!("{}({}) => {}", name, args, err))
         })
     };
 }
@@ -59,14 +59,14 @@ pub(crate) fn metadata<P: AsRef<Path> + Debug>(path: P) -> Result<Metadata> {
 pub(crate) fn mkdir_p<P: AsRef<Path> + Debug>(path: P) -> Result<()> {
     fs::create_dir_all(path.as_ref()).map_err(|err| {
         let err = format!("mkdir_p({:?}) => {}", path.as_ref(), err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
 pub(crate) fn rmdir<P: AsRef<Path> + Debug>(path: P) -> Result<()> {
     fs::remove_dir(path.as_ref()).map_err(|err| {
         let err = format!("rmdir({:?}) => {}", path.as_ref(), err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
@@ -77,21 +77,21 @@ pub(crate) fn chdir<P: AsRef<Path> + Debug>(path: P) -> Result<()> {
 pub(crate) fn touch<P: AsRef<Path> + Debug>(path: P) -> Result<()> {
     File::create(path.as_ref()).map(|_| ()).map_err(|err| {
         let err = format!("touch({:?}) => {}", path.as_ref(), err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
 pub(crate) fn read(fd: RawFd, buf: &mut [u8]) -> Result<usize> {
     unistd::read(fd, buf).map_err(|err| {
         let err = format!("read({:?}, ...) => {}", fd, err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
 pub(crate) fn write(fd: RawFd, buf: &[u8]) -> Result<usize> {
     unistd::write(fd, buf).map_err(|err| {
         let err = format!("write({:?}, ...) => {}", fd, err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
@@ -106,7 +106,7 @@ pub(crate) fn dup2(oldfd: RawFd, newfd: RawFd) -> Result<RawFd> {
 pub(crate) fn fwrite<P: AsRef<Path> + Debug>(path: P, content: &str) -> Result<()> {
     fs::write(path.as_ref(), content.as_bytes()).map_err(|err| {
         let err = format!("write({:?}, ...) => {}", path.as_ref(), err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
@@ -155,7 +155,7 @@ pub(crate) fn unshare(clone_flags: CloneFlags) -> Result<()> {
 pub(crate) fn fork() -> Result<ForkResult> {
     unsafe { unistd::fork() }.map_err(|err| {
         let err = format!("fork() => {}", err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 
@@ -191,7 +191,7 @@ pub(crate) fn prctl_set_pdeathsig(sig: i32) -> Result<()> {
     let res = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, sig, 0, 0, 0) };
     if res == -1 {
         let err = format!("prctl(PR_SET_PDEATHSIG, {:?}, ...) => {}", sig, res);
-        Err(Error(err))
+        Err(Error::SyscallError(err))
     } else {
         Ok(())
     }
@@ -201,7 +201,7 @@ pub(crate) fn prctl_set_no_new_privs() -> Result<()> {
     let res = unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) };
     if res == -1 {
         let err = format!("prctl(PR_SET_NO_NEW_PRIVS, ...) => {}", res);
-        Err(Error(err))
+        Err(Error::SyscallError(err))
     } else {
         Ok(())
     }
@@ -210,7 +210,7 @@ pub(crate) fn prctl_set_no_new_privs() -> Result<()> {
 pub(crate) fn sigaction(signal: Signal, sigaction: &SigAction) -> Result<SigAction> {
     unsafe { signal::sigaction(signal, sigaction) }.map_err(|err| {
         let err = format!("sigaction({:?}, ...) => {}", signal, err);
-        Error(err)
+        Error::SyscallError(err)
     })
 }
 

hakoniwa/src/error.rs

@@ -2,8 +2,8 @@ pub type Result<T> = std::result::Result<T, Error>;
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
-    #[error("{0}")]
-    ParseConfigurationError(String),
+    #[error(transparent)]
+    ParseConfigurationError(#[from] ParseConfigurationErrorKind),
     #[error("{0}: {1}")]
     PathError(std::path::PathBuf, String),
     #[error("seccomp: {0}")]
@@ -12,14 +12,10 @@ pub enum Error {
     _ExecutorRunError(String),
 }
 
-impl From<handlebars::RenderError> for Error {
-    fn from(e: handlebars::RenderError) -> Self {
-        Self::ParseConfigurationError(e.to_string())
-    }
-}
-
-impl From<toml::de::Error> for Error {
-    fn from(e: toml::de::Error) -> Self {
-        Self::ParseConfigurationError(e.to_string())
-    }
+#[derive(thiserror::Error, Debug)]
+pub enum ParseConfigurationErrorKind {
+    #[error(transparent)]
+    HandlebarsRenderError(#[from] handlebars::RenderError),
+    #[error(transparent)]
+    TomlError(#[from] toml::de::Error),
 }

hakoniwa/src/sandbox.rs

@@ -5,7 +5,7 @@ use std::{collections::HashMap, str};
 
 use crate::{
     contrib::handlebars::{fs_read_to_string_helper, os_env_helper, os_homedir_helper},
-    Executor, File, Limits, Mount, Result, Seccomp,
+    error, Executor, File, Limits, Mount, Result, Seccomp,
 };
 
 lazy_static! {
@@ -43,8 +43,11 @@ impl SandboxPolicy {
     /// Create a policy from a string.
     #[allow(clippy::should_implement_trait)]
     pub fn from_str(data: &str) -> Result<Self> {
-        let data = SANDBOX_POLICY_HANDLEBARS.render_template(data, &())?;
-        let policy: Self = toml::from_str(&data)?;
+        let data = SANDBOX_POLICY_HANDLEBARS
+            .render_template(data, &())
+            .map_err(error::ParseConfigurationErrorKind::HandlebarsRenderError)?;
+        let policy: Self =
+            toml::from_str(&data).map_err(error::ParseConfigurationErrorKind::TomlError)?;
         Ok(policy)
     }
 }