From 38070766d15c44c3d731aa5b231e3b2ad698ac05 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Tue, 14 Dec 2021 18:11:04 -0500 Subject: simplify error handling a bunch --- CHANGELOG.md | 6 ++++ src/command.rs | 13 +++------ src/error.rs | 83 ++++++++++++++++++++++++++++++++--------------------- src/lib.rs | 2 +- src/pty.rs | 8 +++--- src/pty/async_io.rs | 10 +++---- src/pty/std.rs | 6 ++-- src/pty/tokio.rs | 12 ++++---- 8 files changed, 76 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b81df12..481e622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Changed + +* Simplified the `Error` type to remove a bunch of unnecessary distinctions + ## [0.1.1] - 2021-11-10 ### Changed 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 for Source { + fn from(e: std::io::Error) -> Self { + Self::Io(e) + } +} - /// error opening pts at \ - OpenPts(std::io::Error, std::path::PathBuf), +impl std::convert::From 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 = std::result::Result; + +pub fn create_pty>(source: S) -> Error { + Error::CreatePty(source.into()) +} + +pub fn set_term_size>(source: S) -> Error { + Error::SetTermSize(source.into()) +} + +pub fn spawn>(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) } } -- cgit v1.2.3-54-g00ecf