simplify error handling a bunch

7 files changed, 70 insertions, 64 deletions

diff --git a/src/command.rs b/src/command.rs
index d810d79..303b885 100644
--- a/src/command.rs
+++ b/src/command.rs
@@ -22,13 +22,9 @@ pub trait Command {
     /// calls to `stdin`/`stdout`/`stderr`.
     ///
     /// # Errors
-    /// * `Error::AsyncPty`: error making pty async
-    /// * `Error::AsyncPtyNix`: error making pty async
     /// * `Error::CreatePty`: error creating pty
-    /// * `Error::OpenPts`: error opening pts
     /// * `Error::SetTermSize`: error setting terminal size
     /// * `Error::Spawn`: error spawning subprocess
-    /// * `Error::SpawnNix`: error spawning subprocess
     fn spawn_pty(
         &mut self,
         size: Option<&crate::pty::Size>,
@@ -81,7 +77,7 @@ where
         // async-signal-safe).
         unsafe { self.pre_exec_impl(pre_exec) };
 
-        let child = self.spawn_impl().map_err(crate::error::Error::Spawn)?;
+        let child = self.spawn_impl().map_err(crate::error::spawn)?;
 
         Ok(Child { child, pty })
     }
@@ -185,12 +181,11 @@ where
     let pts = pty.pts()?;
     let pts_fd = pts.as_raw_fd();
 
-    let stdin =
-        nix::unistd::dup(pts_fd).map_err(crate::error::Error::SpawnNix)?;
+    let stdin = nix::unistd::dup(pts_fd).map_err(crate::error::create_pty)?;
     let stdout =
-        nix::unistd::dup(pts_fd).map_err(crate::error::Error::SpawnNix)?;
+        nix::unistd::dup(pts_fd).map_err(crate::error::create_pty)?;
     let stderr =
-        nix::unistd::dup(pts_fd).map_err(crate::error::Error::SpawnNix)?;
+        nix::unistd::dup(pts_fd).map_err(crate::error::create_pty)?;
 
     Ok((pty, pts, stdin, stdout, stderr))
 }
diff --git a/src/error.rs b/src/error.rs
index 597d692..41ad4a2 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,52 +1,58 @@
-/// Error type for this crate
+/// Source for errors in this crate
 #[derive(Debug)]
-pub enum Error {
-    /// error making pty async
-    AsyncPty(std::io::Error),
+pub enum Source {
+    /// error came from std::io::Error
+    Io(std::io::Error),
+    /// error came from nix::Error
+    Nix(nix::Error),
+}
 
-    /// error making pty async
-    AsyncPtyNix(nix::Error),
+impl std::fmt::Display for Source {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Io(e) => write!(f, "{}", e),
+            Self::Nix(e) => write!(f, "{}", e),
+        }
+    }
+}
 
-    /// error creating pty
-    CreatePty(nix::Error),
+impl std::convert::From<std::io::Error> for Source {
+    fn from(e: std::io::Error) -> Self {
+        Self::Io(e)
+    }
+}
 
-    /// error opening pts at \<path\>
-    OpenPts(std::io::Error, std::path::PathBuf),
+impl std::convert::From<nix::Error> for Source {
+    fn from(e: nix::Error) -> Self {
+        Self::Nix(e)
+    }
+}
 
-    /// error setting terminal size
-    SetTermSize(nix::Error),
+/// Error type for this crate
+#[derive(Debug)]
+pub enum Error {
+    /// error creating pty
+    CreatePty(Source),
 
-    /// error spawning subprocess
-    Spawn(std::io::Error),
+    /// error setting terminal size
+    SetTermSize(Source),
 
     /// error spawning subprocess
-    SpawnNix(nix::Error),
+    Spawn(Source),
 }
 
 impl std::fmt::Display for Error {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::AsyncPty(e) => {
-                write!(f, "error making pty async: {}", e)
-            }
-            Self::AsyncPtyNix(e) => {
-                write!(f, "error making pty async: {}", e)
-            }
             Self::CreatePty(e) => {
                 write!(f, "error creating pty: {}", e)
             }
-            Self::OpenPts(e, path) => {
-                write!(f, "error opening pts at {}: {}", path.display(), e)
-            }
             Self::SetTermSize(e) => {
                 write!(f, "error setting terminal size: {}", e)
             }
             Self::Spawn(e) => {
                 write!(f, "error spawning subprocess: {}", e)
             }
-            Self::SpawnNix(e) => {
-                write!(f, "error spawning subprocess: {}", e)
-            }
         }
     }
 }
@@ -54,16 +60,27 @@ impl std::fmt::Display for Error {
 impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
-            Self::AsyncPty(e) | Self::Spawn(e) | Self::OpenPts(e, _) => {
-                Some(e)
-            }
-            Self::AsyncPtyNix(e) | Self::SpawnNix(e) | Self::CreatePty(e) => {
-                Some(e)
+            Self::CreatePty(e) | Self::SetTermSize(e) | Self::Spawn(e) => {
+                match e {
+                    Source::Io(e) => Some(e),
+                    Source::Nix(e) => Some(e),
+                }
             }
-            Self::SetTermSize(e) => Some(e),
         }
     }
 }
 
 /// Convenience wrapper for `Result`s using [`Error`](Error)
 pub type Result<T> = std::result::Result<T, Error>;
+
+pub fn create_pty<S: std::convert::Into<Source>>(source: S) -> Error {
+    Error::CreatePty(source.into())
+}
+
+pub fn set_term_size<S: std::convert::Into<Source>>(source: S) -> Error {
+    Error::SetTermSize(source.into())
+}
+
+pub fn spawn<S: std::convert::Into<Source>>(source: S) -> Error {
+    Error::Spawn(source.into())
+}
diff --git a/src/lib.rs b/src/lib.rs
index 5d98841..838706b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -62,7 +62,7 @@
 mod command;
 pub use command::{Child, Command};
 mod error;
-pub use error::{Error, Result};
+pub use error::{Error, Result, Source};
 mod pty;
 pub use pty::Size;
 
diff --git a/src/pty.rs b/src/pty.rs
index fde86b1..a569f88 100644
--- a/src/pty.rs
+++ b/src/pty.rs
@@ -75,12 +75,12 @@ fn create_pt(
     let pt = nix::pty::posix_openpt(
         nix::fcntl::OFlag::O_RDWR | nix::fcntl::OFlag::O_NOCTTY,
     )
-    .map_err(crate::error::Error::CreatePty)?;
-    nix::pty::grantpt(&pt).map_err(crate::error::Error::CreatePty)?;
-    nix::pty::unlockpt(&pt).map_err(crate::error::Error::CreatePty)?;
+    .map_err(crate::error::create_pty)?;
+    nix::pty::grantpt(&pt).map_err(crate::error::create_pty)?;
+    nix::pty::unlockpt(&pt).map_err(crate::error::create_pty)?;
 
     let ptsname = nix::pty::ptsname_r(&pt)
-        .map_err(crate::error::Error::CreatePty)?
+        .map_err(crate::error::create_pty)?
         .into();
 
     let pt_fd = pt.into_raw_fd();
diff --git a/src/pty/async_io.rs b/src/pty/async_io.rs
index 985b2af..304f403 100644
--- a/src/pty/async_io.rs
+++ b/src/pty/async_io.rs
@@ -18,8 +18,8 @@ impl super::Pty for Pty {
         // File object to take full ownership.
         let pt = unsafe { std::fs::File::from_raw_fd(pt_fd) };
 
-        let pt = async_io::Async::new(pt)
-            .map_err(crate::error::Error::AsyncPty)?;
+        let pt =
+            async_io::Async::new(pt).map_err(crate::error::create_pty)?;
 
         Ok(Self { pt, ptsname })
     }
@@ -37,14 +37,12 @@ impl super::Pty for Pty {
             .read(true)
             .write(true)
             .open(&self.ptsname)
-            .map_err(|e| {
-                crate::error::Error::OpenPts(e, self.ptsname.clone())
-            })?;
+            .map_err(crate::error::create_pty)?;
         Ok(fh)
     }
 
     fn resize(&self, size: &super::Size) -> crate::error::Result<()> {
         super::set_term_size(self.pt().as_raw_fd(), size)
-            .map_err(crate::error::Error::SetTermSize)
+            .map_err(crate::error::set_term_size)
     }
 }
diff --git a/src/pty/std.rs b/src/pty/std.rs
index 612c9b6..c907052 100644
--- a/src/pty/std.rs
+++ b/src/pty/std.rs
@@ -34,14 +34,12 @@ impl super::Pty for Pty {
             .read(true)
             .write(true)
             .open(&self.ptsname)
-            .map_err(|e| {
-                crate::error::Error::OpenPts(e, self.ptsname.clone())
-            })?;
+            .map_err(crate::error::create_pty)?;
         Ok(fh)
     }
 
     fn resize(&self, size: &super::Size) -> crate::error::Result<()> {
         super::set_term_size(self.pt().as_raw_fd(), size)
-            .map_err(crate::error::Error::SetTermSize)
+            .map_err(crate::error::set_term_size)
     }
 }
diff --git a/src/pty/tokio.rs b/src/pty/tokio.rs
index 116ec6e..573d73b 100644
--- a/src/pty/tokio.rs
+++ b/src/pty/tokio.rs
@@ -104,7 +104,7 @@ impl super::Pty for Pty {
         let (pt_fd, ptsname) = super::create_pt()?;
 
         let bits = nix::fcntl::fcntl(pt_fd, nix::fcntl::FcntlArg::F_GETFL)
-            .map_err(crate::error::Error::AsyncPtyNix)?;
+            .map_err(crate::error::create_pty)?;
         // this should be safe because i am just using the return value of
         // F_GETFL directly, but for whatever reason nix doesn't like
         // from_bits(bits) (it claims it has an unknown field)
@@ -114,7 +114,7 @@ impl super::Pty for Pty {
             )
         };
         nix::fcntl::fcntl(pt_fd, nix::fcntl::FcntlArg::F_SETFL(opts))
-            .map_err(crate::error::Error::AsyncPtyNix)?;
+            .map_err(crate::error::create_pty)?;
 
         // safe because posix_openpt (or the previous functions operating on
         // the result) would have returned an Err (causing us to return early)
@@ -125,7 +125,7 @@ impl super::Pty for Pty {
 
         let pt = AsyncPty(
             tokio::io::unix::AsyncFd::new(pt)
-                .map_err(crate::error::Error::AsyncPty)?,
+                .map_err(crate::error::create_pty)?,
         );
 
         Ok(Self { pt, ptsname })
@@ -144,14 +144,12 @@ impl super::Pty for Pty {
             .read(true)
             .write(true)
             .open(&self.ptsname)
-            .map_err(|e| {
-                crate::error::Error::OpenPts(e, self.ptsname.clone())
-            })?;
+            .map_err(crate::error::create_pty)?;
         Ok(fh)
     }
 
     fn resize(&self, size: &super::Size) -> crate::error::Result<()> {
         super::set_term_size(self.pt().as_raw_fd(), size)
-            .map_err(crate::error::Error::SetTermSize)
+            .map_err(crate::error::set_term_size)
     }
 }