Skip to content

Commit

Permalink
Add an error resource to WASI streams (#7152)
Browse files Browse the repository at this point in the history
* Change `bindgen!`'s trappable error to take an input type

This commit removes the generated type for `trappable_error_type` from
the `bindgen!` macro to allow users to provide a type instead. This
works similarly as before with new conversion functions generated in
traits which are used to convert the custom error into the ABI
representation of what a WIT world expects.

There are a few motivations for this commit:

* This enables reducing the number of errors in play between async/sync
  bindings by using the same error type there.

* This avoids an auto-generated type which is more difficult to inspect
  than one that's written down already and the source can more easily be
  inspected.

* This enables richer conversions using `self` (e.g. `self.table_mut()`)
  between error types rather than purely relying on `Into`. This is
  important for #7017 where an error is going to be inserted into the
  table as it gets converted.

* Fix tests

* Update WASI to use new trappable errors

This commit deals with the fallout of the previous commit for the WASI
preview2 implementation. The main changes here are:

* Bindgen-generated `Error` types no longer exist. These are replaced
  with `TrappableError<T>` where type aliases are used such as

  ```rust
  type FsError = TrappableError<wasi::filesystem::types::ErrorCode>;
  ```

* Type synonyms such as `FsResult<T>` are now added for more
  conveniently writing down fallible signatures.

* Some various error conversions are updated from converting to the old
  `Error` type to now instead directly into corresponding `ErrorCode` types.

* A number of cases where unknown errors were turned into traps now
  return bland error codes and log the error instead since these aren't
  fatal events.

* The `StreamError` type does not use `TrappableError` since it has
  other variants that it's concerned with such as a
  `LastOperationFailed` variant which has an `anyhow::Error` payload.

* Some minor preview1 issues were fixed such as trapping errors being
  turned into normal I/O errors by accident.

* Add an `error` resource to WASI streams

This commit adds a new `error` resource to the `wasi:io/streams`
interface. This `error` resource is returned as part of
`last-operation-failed` and serves as a means to discover through other
interfaces more granular type information than a generic string. This
error type has a new function in the `filesystem` interface, for
example, which enables getting filesystem-related error codes from I/O
performed on filesystem-originating streams. This is plumbed through to
the adapter as well to return more than `ERRNO_IO` from failed
read/write operations now too.

This is not super fancy just yet but is intended to be a vector through
which future additions can be done. The main thing this enables is to
avoid dropping errors on the floor in the host and enabling the guest to
discover further information about I/O errors on streams.

Closes #7017

* Update crates/wasi-http/wit/deps/io/streams.wit

Co-authored-by: Trevor Elliott <awesomelyawesome@gmail.com>

* Update wasi-http wit too

* Remove unnecessary clone

---------

Co-authored-by: Trevor Elliott <awesomelyawesome@gmail.com>
  • Loading branch information
alexcrichton and elliottt authored Oct 5, 2023
1 parent df15006 commit 722310a
Show file tree
Hide file tree
Showing 23 changed files with 825 additions and 639 deletions.
5 changes: 3 additions & 2 deletions crates/component-macro/src/bindgen.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use proc_macro2::{Span, TokenStream};
use quote::ToTokens;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use syn::parse::{Error, Parse, ParseStream, Result};
use syn::punctuated::Punctuated;
use syn::{braced, token, Ident, Token};
use syn::{braced, token, Token};
use wasmtime_wit_bindgen::{AsyncConfig, Opts, Ownership, TrappableError};
use wit_parser::{PackageId, Resolve, UnresolvedPackage, WorldId};

Expand Down Expand Up @@ -335,7 +336,7 @@ fn trappable_error_field_parse(input: ParseStream<'_>) -> Result<TrappableError>
input.parse::<Token![::]>()?;
let wit_type_name = ident_or_str(input)?;
input.parse::<Token![:]>()?;
let rust_type_name = input.parse::<Ident>()?.to_string();
let rust_type_name = input.parse::<syn::Path>()?.to_token_stream().to_string();
Ok(TrappableError {
wit_package_path,
wit_type_name,
Expand Down
42 changes: 42 additions & 0 deletions crates/component-macro/tests/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,45 @@ mod with_key_and_resources {
}
}
}

mod trappable_errors {
wasmtime::component::bindgen!({
inline: "
package demo:pkg;
interface a {
type b = u64;
z1: func() -> result<_, b>;
z2: func() -> result<_, b>;
}
interface b {
use a.{b};
z: func() -> result<_, b>;
}
interface c {
type b = u64;
}
interface d {
use c.{b};
z: func() -> result<_, b>;
}
world foo {
import a;
import b;
import d;
}
",
trappable_error_type: {
"demo:pkg/a"::"b": MyX,
"demo:pkg/c"::"b": MyX,
},
});

#[allow(dead_code)]
type MyX = u32;
}
14 changes: 13 additions & 1 deletion crates/wasi-http/wit/deps/filesystem/types.wit
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
///
/// [WASI filesystem path resolution]: https://github.com/WebAssembly/wasi-filesystem/blob/main/path-resolution.md
interface types {
use wasi:io/streams.{input-stream, output-stream}
use wasi:io/streams.{input-stream, output-stream, error}
use wasi:clocks/wall-clock.{datetime}

/// File size or length of a region within a file.
Expand Down Expand Up @@ -795,4 +795,16 @@ interface types {
/// Read a single directory entry from a `directory-entry-stream`.
read-directory-entry: func() -> result<option<directory-entry>, error-code>
}

/// Attempts to extract a filesystem-related `error-code` from the stream
/// `error` provided.
///
/// Stream operations which return `stream-error::last-operation-failed`
/// have a payload with more information about the operation that failed.
/// This payload can be passed through to this function to see if there's
/// filesystem-related information about the error to return.
///
/// Note that this function is fallible because not all stream-related
/// errors are filesystem-related errors.
filesystem-error-code: func(err: borrow<error>) -> option<error-code>
}
26 changes: 24 additions & 2 deletions crates/wasi-http/wit/deps/io/streams.wit
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,37 @@ interface streams {
use poll.{pollable}

/// An error for input-stream and output-stream operations.
enum stream-error {
variant stream-error {
/// The last operation (a write or flush) failed before completion.
last-operation-failed,
///
/// More information is available in the `error` payload.
last-operation-failed(error),
/// The stream is closed: no more input will be accepted by the
/// stream. A closed output-stream will return this error on all
/// future operations.
closed
}

/// Contextual error information about the last failure that happened on
/// a read, write, or flush from an `input-stream` or `output-stream`.
///
/// This type is returned through the `stream-error` type whenever an
/// operation on a stream directly fails or an error is discovered
/// after-the-fact, for example when a write's failure shows up through a
/// later `flush` or `check-write`.
///
/// Interfaces such as `wasi:filesystem/types` provide functionality to
/// further "downcast" this error into interface-specific error information.
resource error {
/// Returns a string that's suitable to assist humans in debugging this
/// error.
///
/// The returned string will change across platforms and hosts which
/// means that parsing it, for example, would be a
/// platform-compatibility hazard.
to-debug-string: func() -> string
}

/// An input bytestream.
///
/// `input-stream`s are *non-blocking* to the extent practical on underlying
Expand Down
28 changes: 23 additions & 5 deletions crates/wasi-preview1-component-adapter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,9 @@ pub unsafe extern "C" fn fd_read(
*nread = 0;
return Ok(());
}
Err(_) => Err(ERRNO_IO)?,
Err(streams::StreamError::LastOperationFailed(e)) => {
Err(stream_error_to_errno(e))?
}
};

assert_eq!(data.as_ptr(), ptr);
Expand All @@ -925,6 +927,13 @@ pub unsafe extern "C" fn fd_read(
})
}

fn stream_error_to_errno(err: streams::Error) -> Errno {
match filesystem::filesystem_error_code(&err) {
Some(code) => code.into(),
None => ERRNO_IO,
}
}

/// Read directory entries from a directory.
/// When successful, the contents of the output buffer consist of a sequence of
/// directory entries. Each directory entry consists of a `dirent` object,
Expand Down Expand Up @@ -2160,7 +2169,10 @@ impl BlockingMode {
bytes = rest;
match output_stream.blocking_write_and_flush(chunk) {
Ok(()) => {}
Err(_) => return Err(ERRNO_IO),
Err(streams::StreamError::Closed) => return Err(ERRNO_IO),
Err(streams::StreamError::LastOperationFailed(e)) => {
return Err(stream_error_to_errno(e))
}
}
}
Ok(total)
Expand All @@ -2170,7 +2182,9 @@ impl BlockingMode {
let permit = match output_stream.check_write() {
Ok(n) => n,
Err(streams::StreamError::Closed) => 0,
Err(streams::StreamError::LastOperationFailed) => return Err(ERRNO_IO),
Err(streams::StreamError::LastOperationFailed(e)) => {
return Err(stream_error_to_errno(e))
}
};

let len = bytes.len().min(permit as usize);
Expand All @@ -2181,13 +2195,17 @@ impl BlockingMode {
match output_stream.write(&bytes[..len]) {
Ok(_) => {}
Err(streams::StreamError::Closed) => return Ok(0),
Err(streams::StreamError::LastOperationFailed) => return Err(ERRNO_IO),
Err(streams::StreamError::LastOperationFailed(e)) => {
return Err(stream_error_to_errno(e))
}
}

match output_stream.blocking_flush() {
Ok(_) => {}
Err(streams::StreamError::Closed) => return Ok(0),
Err(streams::StreamError::LastOperationFailed) => return Err(ERRNO_IO),
Err(streams::StreamError::LastOperationFailed(e)) => {
return Err(stream_error_to_errno(e))
}
}

Ok(len)
Expand Down
8 changes: 0 additions & 8 deletions crates/wasi/src/preview2/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ wasmtime::component::bindgen!({
world: "wasi:cli/command",
tracing: true,
async: true,
trappable_error_type: {
"wasi:filesystem/types"::"error-code": Error,
"wasi:sockets/tcp"::"error-code": Error,
},
with: {
"wasi:filesystem/types": crate::preview2::bindings::filesystem::types,
"wasi:filesystem/preopens": crate::preview2::bindings::filesystem::preopens,
Expand Down Expand Up @@ -65,10 +61,6 @@ pub mod sync {
world: "wasi:cli/command",
tracing: true,
async: false,
trappable_error_type: {
"wasi:filesystem/types"::"error-code": Error,
"wasi:sockets/tcp"::"error-code": Error,
},
with: {
"wasi:filesystem/types": crate::preview2::bindings::sync_io::filesystem::types,
"wasi:filesystem/preopens": crate::preview2::bindings::filesystem::preopens,
Expand Down
74 changes: 74 additions & 0 deletions crates/wasi/src/preview2/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::error::Error;
use std::fmt;
use std::marker;

/// An error returned from the `proc_exit` host syscall.
///
Expand All @@ -14,3 +16,75 @@ impl fmt::Display for I32Exit {
}

impl std::error::Error for I32Exit {}

/// A helper error type used by many other modules through type aliases.
///
/// This type is an `Error` itself and is intended to be a representation of
/// either:
///
/// * A custom error type `T`
/// * A trap, represented as `anyhow::Error`
///
/// This error is created through either the `::trap` constructor representing a
/// full-fledged trap or the `From<T>` constructor which is intended to be used
/// with `?`. The goal is to make normal errors `T` "automatic" but enable error
/// paths to return a `::trap` error optionally still as necessary without extra
/// boilerplate everywhere else.
///
/// Note that this type isn't used directly but instead is intended to be used
/// as:
///
/// ```rust,ignore
/// type MyError = TrappableError<bindgen::TheError>;
/// ```
///
/// where `MyError` is what you'll use throughout bindings code and
/// `bindgen::TheError` is the type that this represents as generated by the
/// `bindgen!` macro.
#[repr(transparent)]
pub struct TrappableError<T> {
err: anyhow::Error,
_marker: marker::PhantomData<T>,
}

impl<T> TrappableError<T> {
pub fn trap(err: impl Into<anyhow::Error>) -> TrappableError<T> {
TrappableError {
err: err.into(),
_marker: marker::PhantomData,
}
}

pub fn downcast(self) -> anyhow::Result<T>
where
T: Error + Send + Sync + 'static,
{
self.err.downcast()
}
}

impl<T> From<T> for TrappableError<T>
where
T: Error + Send + Sync + 'static,
{
fn from(error: T) -> Self {
Self {
err: error.into(),
_marker: marker::PhantomData,
}
}
}

impl<T> fmt::Debug for TrappableError<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.err.fmt(f)
}
}

impl<T> fmt::Display for TrappableError<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.err.fmt(f)
}
}

impl<T> Error for TrappableError<T> {}
29 changes: 22 additions & 7 deletions crates/wasi/src/preview2/filesystem.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,30 @@
use crate::preview2::bindings::filesystem::types;
use crate::preview2::{
spawn_blocking, AbortOnDropJoinHandle, HostOutputStream, StreamError, Subscribe,
spawn_blocking, AbortOnDropJoinHandle, HostOutputStream, StreamError, Subscribe, TableError,
TrappableError,
};
use anyhow::anyhow;
use bytes::{Bytes, BytesMut};
use std::io;
use std::mem;
use std::sync::Arc;

pub type FsResult<T> = Result<T, FsError>;

pub type FsError = TrappableError<types::ErrorCode>;

impl From<TableError> for FsError {
fn from(error: TableError) -> Self {
Self::trap(error)
}
}

impl From<io::Error> for FsError {
fn from(error: io::Error) -> Self {
types::ErrorCode::from(error).into()
}
}

pub enum Descriptor {
File(File),
Dir(Dir),
Expand Down Expand Up @@ -276,24 +293,22 @@ impl Subscribe for FileOutputStream {
}

pub struct ReaddirIterator(
std::sync::Mutex<
Box<dyn Iterator<Item = Result<types::DirectoryEntry, types::Error>> + Send + 'static>,
>,
std::sync::Mutex<Box<dyn Iterator<Item = FsResult<types::DirectoryEntry>> + Send + 'static>>,
);

impl ReaddirIterator {
pub(crate) fn new(
i: impl Iterator<Item = Result<types::DirectoryEntry, types::Error>> + Send + 'static,
i: impl Iterator<Item = FsResult<types::DirectoryEntry>> + Send + 'static,
) -> Self {
ReaddirIterator(std::sync::Mutex::new(Box::new(i)))
}
pub(crate) fn next(&self) -> Result<Option<types::DirectoryEntry>, types::Error> {
pub(crate) fn next(&self) -> FsResult<Option<types::DirectoryEntry>> {
self.0.lock().unwrap().next().transpose()
}
}

impl IntoIterator for ReaddirIterator {
type Item = Result<types::DirectoryEntry, types::Error>;
type Item = FsResult<types::DirectoryEntry>;
type IntoIter = Box<dyn Iterator<Item = Self::Item> + Send>;

fn into_iter(self) -> Self::IntoIter {
Expand Down
Loading

0 comments on commit 722310a

Please sign in to comment.