Use wiggle "trappable error" to implement wasi-common

crates/wasi-common/cap-std-sync/src/dir.rs

@@ -138,14 +138,27 @@ impl WasiDir for Dir {
         &self,
         cursor: ReaddirCursor,
     ) -> Result<Box<dyn Iterator<Item = Result<ReaddirEntity, Error>> + Send>, Error> {
+        // We need to keep a full-fidelity io Error around to check for a special failure mode
+        // on windows, but also this function can fail due to an illegal byte sequence in a
+        // filename, which we can't construct an io Error to represent.
+        enum ReaddirError {
+            Io(std::io::Error),
+            IllegalSequence,
+        }
+        impl From<std::io::Error> for ReaddirError {
+            fn from(e: std::io::Error) -> ReaddirError {
+                ReaddirError::Io(e)
+            }
+        }
+
         // cap_std's read_dir does not include . and .., we should prepend these.
         // Why does the Ok contain a tuple? We can't construct a cap_std::fs::DirEntry, and we don't
         // have enough info to make a ReaddirEntity yet.
         let dir_meta = self.0.dir_metadata()?;
         let rd = vec![
             {
                 let name = ".".to_owned();
-                Ok((FileType::Directory, dir_meta.ino(), name))
+                Ok::<_, ReaddirError>((FileType::Directory, dir_meta.ino(), name))
             },
             {
                 let name = "..".to_owned();
@@ -163,24 +176,22 @@ impl WasiDir for Dir {
                 let name = entry
                     .file_name()
                     .into_string()
-                    .map_err(|_| Error::illegal_byte_sequence().context("filename"))?;
+                    .map_err(|_| ReaddirError::IllegalSequence)?;
                 Ok((filetype, inode, name))
             });
 
             // On Windows, filter out files like `C:\DumpStack.log.tmp` which we
             // can't get a full metadata for.
             #[cfg(windows)]
-            let entries = entries.filter(|entry: &Result<_, wasi_common::Error>| {
+            let entries = entries.filter(|entry| {
                 use windows_sys::Win32::Foundation::{
                     ERROR_ACCESS_DENIED, ERROR_SHARING_VIOLATION,
                 };
-                if let Err(err) = entry {
-                    if let Some(err) = err.downcast_ref::<std::io::Error>() {
-                        if err.raw_os_error() == Some(ERROR_SHARING_VIOLATION as i32)
-                            || err.raw_os_error() == Some(ERROR_ACCESS_DENIED as i32)
-                        {
-                            return false;
-                        }
+                if let Err(ReaddirError::Io(err)) = entry {
+                    if err.raw_os_error() == Some(ERROR_SHARING_VIOLATION as i32)
+                        || err.raw_os_error() == Some(ERROR_ACCESS_DENIED as i32)
+                    {
+                        return false;
                     }
                 }
                 true
@@ -197,7 +208,8 @@ impl WasiDir for Dir {
                 inode,
                 name,
             }),
-            Err(e) => Err(e),
+            Err(ReaddirError::Io(e)) => Err(e.into()),
+            Err(ReaddirError::IllegalSequence) => Err(Error::illegal_byte_sequence()),
         })
         .skip(u64::from(cursor) as usize);
 

crates/wasi-common/cap-std-sync/src/sched/unix.rs

@@ -47,7 +47,7 @@ pub async fn poll_oneoff<'a>(poll: &mut Poll<'a>) -> Result<(), Error> {
         match rustix::io::poll(&mut pollfds, poll_timeout) {
             Ok(ready) => break ready,
             Err(rustix::io::Errno::INTR) => continue,
-            Err(err) => return Err(err.into()),
+            Err(err) => return Err(std::io::Error::from(err).into()),
         }
     };
     if ready > 0 {

crates/wasi-common/cap-std-sync/src/sched/windows.rs

@@ -8,7 +8,6 @@
 // We suspect there are bugs in this scheduler, however, we have not
 // taken the time to improve it. See bug #2880.
 
-use anyhow::Context;
 use once_cell::sync::Lazy;
 use std::ops::Deref;
 use std::sync::mpsc::{self, Receiver, RecvTimeoutError, Sender, TryRecvError};
@@ -73,7 +72,7 @@ pub async fn poll_oneoff_<'a>(
     if !stdin_read_subs.is_empty() {
         let state = STDIN_POLL
             .lock()
-            .map_err(|_| Error::trap("failed to take lock of STDIN_POLL"))?
+            .map_err(|_| Error::trap(anyhow::Error::msg("failed to take lock of STDIN_POLL")))?
             .poll(waitmode)?;
         for readsub in stdin_read_subs.into_iter() {
             match state {
@@ -167,34 +166,36 @@ impl StdinPoll {
             // Clean up possibly unread result from previous poll.
             Ok(_) | Err(TryRecvError::Empty) => {}
             Err(TryRecvError::Disconnected) => {
-                return Err(Error::trap("StdinPoll notify_rx channel closed"))
+                return Err(Error::trap(anyhow::Error::msg(
+                    "StdinPoll notify_rx channel closed",
+                )))
             }
         }
 
         // Notify the worker thread to poll stdin
         self.request_tx
             .send(())
-            .context("request_tx channel closed")?;
+            .map_err(|_| Error::trap(anyhow::Error::msg("request_tx channel closed")))?;
 
         // Wait for the worker thread to send a readiness notification
         match wait_mode {
             WaitMode::Timeout(timeout) => match self.notify_rx.recv_timeout(timeout) {
                 Ok(r) => Ok(r),
                 Err(RecvTimeoutError::Timeout) => Ok(PollState::TimedOut),
-                Err(RecvTimeoutError::Disconnected) => {
-                    Err(Error::trap("StdinPoll notify_rx channel closed"))
-                }
+                Err(RecvTimeoutError::Disconnected) => Err(Error::trap(anyhow::Error::msg(
+                    "StdinPoll notify_rx channel closed",
+                ))),
             },
             WaitMode::Infinite => self
                 .notify_rx
                 .recv()
-                .context("StdinPoll notify_rx channel closed"),
+                .map_err(|_| Error::trap(anyhow::Error::msg("StdinPoll notify_rx channel closed"))),
             WaitMode::Immediate => match self.notify_rx.try_recv() {
                 Ok(r) => Ok(r),
                 Err(TryRecvError::Empty) => Ok(PollState::NotReady),
-                Err(TryRecvError::Disconnected) => {
-                    Err(Error::trap("StdinPoll notify_rx channel closed"))
-                }
+                Err(TryRecvError::Disconnected) => Err(Error::trap(anyhow::Error::msg(
+                    "StdinPoll notify_rx channel closed",
+                ))),
             },
         }
     }

crates/wasi-common/cap-std-sync/src/stdio.rs

@@ -128,7 +128,9 @@ macro_rules! wasi_file_write_impl {
             }
             async fn write_vectored<'a>(&mut self, bufs: &[io::IoSlice<'a>]) -> Result<u64, Error> {
                 let n = (&*self.0.as_filelike_view::<File>()).write_vectored(bufs)?;
-                Ok(n.try_into().map_err(|c| Error::range().context(c))?)
+                Ok(n.try_into().map_err(|_| {
+                    Error::range().context("converting write_vectored total length")
+                })?)
             }
             async fn write_vectored_at<'a>(
                 &mut self,

crates/wasi-common/src/error.rs

@@ -1,145 +1,15 @@
-//! `wasi_common::Error` is now `anyhow::Error`.
+//! wasi-common uses an [`Error`] type which represents either a preview 1 [`Errno`] enum, on
+//! [`anyhow::Error`] for trapping execution.
 //!
-//! Snapshots (right now only `wasi_common::snapshots::preview_1`) contains
-//! all of the logic for transforming an `Error` into the snapshot's own
-//! `Errno`. They may do so by downcasting the error into any of:
-//! * `std::io::Error` - these are thrown by `std`, `cap_std`, etc for most of
-//! the operations WASI is concerned with.
-//! * `wasi_common::ErrorKind` - these are a subset of the Errnos, and are
-//! constructed directly by wasi-common or an impl rather than coming from the
-//! OS or some library which doesn't know about WASI.
-//! * `wiggle::GuestError`
-//! * `std::num::TryFromIntError`
-//! * `std::str::Utf8Error`
-//! and then applying specialized logic to translate each of those into
-//! `Errno`s.
-//!
-//! The `wasi_common::ErrorExt` trait provides human-friendly constructors for
-//! the `wasi_common::ErrorKind` variants .
-//!
-//! If you throw an error that does not downcast to one of those, it will turn
-//! into a `wiggle::Trap` and terminate execution.
-//!
-//! The real value of using `anyhow::Error` here is being able to use
-//! `anyhow::Result::context` to aid in debugging of errors.
+//! The user can construct an [`Error`] out of an [`Errno`] using the `From`/`Into` traits.
+//! They may also use [`Error::trap`] to construct an error that traps execution. The contents
+//! can be inspected with [`Error::downcast`] and [`Error::downcast_ref`]. Additional context
+//! can be provided with the [`Error::context`] method. This context is only observable with the
+//! `Display` and `Debug` impls of the error.
 
-pub use anyhow::{Context, Error};
+pub use crate::snapshots::preview_1::error::{Errno, Error, ErrorExt};
 use std::fmt;
 
-/// Internal error type for the `wasi-common` crate.
-///
-/// This Contains variants of the WASI `$errno` type that are used internally
-/// by the crate, and which aren't one-to-one with a `std::io::ErrorKind`
-/// error.
-///
-/// When the Rust [io_error_more] feature is stabilized, that will enable
-/// us to replace several more of these codes with `std::io::ErrorKind` codes.
-///
-/// [io_error_more]: https://doc.rust-lang.org/beta/unstable-book/library-features/io-error-more.html
-#[derive(Copy, Clone, Debug, PartialEq, Eq, thiserror::Error)]
-#[non_exhaustive]
-pub enum ErrorKind {
-    /// Errno::TooBig: Argument list too long
-    #[error("TooBig: Argument list too long")]
-    TooBig,
-    /// Errno::Badf: Bad file descriptor
-    #[error("Badf: Bad file descriptor")]
-    Badf,
-    /// Errno::Ilseq: Illegal byte sequence
-    #[error("Ilseq: Illegal byte sequence")]
-    Ilseq,
-    /// Errno::Io: I/O error
-    #[error("Io: I/O error")]
-    Io,
-    /// Errno::Nametoolong: Filename too long
-    #[error("Nametoolong: Filename too long")]
-    Nametoolong,
-    /// Errno::Notdir: Not a directory or a symbolic link to a directory.
-    #[error("Notdir: Not a directory or a symbolic link to a directory")]
-    Notdir,
-    /// Errno::Notsup: Not supported, or operation not supported on socket.
-    #[error("Notsup: Not supported, or operation not supported on socket")]
-    Notsup,
-    /// Errno::Overflow: Value too large to be stored in data type.
-    #[error("Overflow: Value too large to be stored in data type")]
-    Overflow,
-    /// Errno::Range: Result too large
-    #[error("Range: Result too large")]
-    Range,
-    /// Errno::Spipe: Invalid seek
-    #[error("Spipe: Invalid seek")]
-    Spipe,
-    /// Errno::Perm: Permission denied
-    #[error("Permission denied")]
-    Perm,
-}
-
-pub trait ErrorExt {
-    fn trap(msg: impl Into<String>) -> Self;
-    fn not_found() -> Self;
-    fn too_big() -> Self;
-    fn badf() -> Self;
-    fn exist() -> Self;
-    fn illegal_byte_sequence() -> Self;
-    fn invalid_argument() -> Self;
-    fn io() -> Self;
-    fn name_too_long() -> Self;
-    fn not_dir() -> Self;
-    fn not_supported() -> Self;
-    fn overflow() -> Self;
-    fn range() -> Self;
-    fn seek_pipe() -> Self;
-    fn perm() -> Self;
-}
-
-impl ErrorExt for Error {
-    fn trap(msg: impl Into<String>) -> Self {
-        anyhow::anyhow!(msg.into())
-    }
-    fn not_found() -> Self {
-        std::io::Error::from(std::io::ErrorKind::NotFound).into()
-    }
-    fn too_big() -> Self {
-        ErrorKind::TooBig.into()
-    }
-    fn badf() -> Self {
-        ErrorKind::Badf.into()
-    }
-    fn exist() -> Self {
-        std::io::Error::from(std::io::ErrorKind::AlreadyExists).into()
-    }
-    fn illegal_byte_sequence() -> Self {
-        ErrorKind::Ilseq.into()
-    }
-    fn invalid_argument() -> Self {
-        std::io::Error::from(std::io::ErrorKind::InvalidInput).into()
-    }
-    fn io() -> Self {
-        ErrorKind::Io.into()
-    }
-    fn name_too_long() -> Self {
-        ErrorKind::Nametoolong.into()
-    }
-    fn not_dir() -> Self {
-        ErrorKind::Notdir.into()
-    }
-    fn not_supported() -> Self {
-        ErrorKind::Notsup.into()
-    }
-    fn overflow() -> Self {
-        ErrorKind::Overflow.into()
-    }
-    fn range() -> Self {
-        ErrorKind::Range.into()
-    }
-    fn seek_pipe() -> Self {
-        ErrorKind::Spipe.into()
-    }
-    fn perm() -> Self {
-        ErrorKind::Perm.into()
-    }
-}
-
 /// An error returned from the `proc_exit` host syscall.
 ///
 /// Embedders can test if an error returned from wasm is this error, in which

crates/wasi-common/src/lib.rs

@@ -66,7 +66,7 @@ pub use cap_rand::RngCore;
 pub use clocks::{SystemTimeSpec, WasiClocks, WasiMonotonicClock, WasiSystemClock};
 pub use ctx::WasiCtx;
 pub use dir::WasiDir;
-pub use error::{Context, Error, ErrorExt, ErrorKind, I32Exit};
+pub use error::{Error, ErrorExt, I32Exit};
 pub use file::WasiFile;
 pub use sched::{Poll, WasiSched};
 pub use string_array::StringArrayError;