Add structured error types for programmatic consumption

nicholastmosher commented 3 years ago

Feature

The top-level wasmtime crate, used for embedding the wasmtime runtime into Rust applications, should expose structured error types so that callers may react to specific errors that occur within the runtime. Currently, the wasmtime API exposes possible failures as an anyhow::Result, which is akin to returning a String as an error. The conventional wisdom is that while anyhow is great for applications consuming errors, it is more useful for libraries (which produce errors) to define structured errors as enums, potentially using helper crates like thiserror. This is even noted in the anyhow repo:

Use Anyhow if you don't care what error type your functions return, you just want it to be easy. This is common in application code. Use thiserror if you are a library that wants to design your own dedicated error type(s) so that on failures the caller gets exactly the information that you choose.

Benefit

Adding structured errors to the wasmtime API will allow for programmatic users to define logic to react to precise error conditions, without resorting to parsing error strings. A specific use-case that I would benefit from having is being able to recognize when a module is missing a function export.

Implementation

It would be great to have one top-level wasmtime::Error error enum that divided the error space into discrete variants. The thiserror macro could be used to assist in generating the Display implementation for this type.

A brief search through the crates/wasmtime/ directory shows the following number of hits for bail! and anyhow!:

src/types/matching.rs:
- bail!: 24
src/module/serialization.rs:
- bail!: 17
- anyhow!: 3
src/externals.rs:
- bail! 9
- anyhow!: 2
src/linker.rs:
- bail! 8
- anyhow!: 1
src/config.rs:
- bail!: 5
- anyhow!: 1
src/instance.rs:
- bail! 5
- anyhow!: 1
src/values.rs:
- bail!: 4
src/func/typed.rs:
- bail!: 3
src/func.rs:
- bail!: 3
src/store.rs:
- bail!: 2
src/trampoline/memory.rs:
- anyhow!: 1

This amounts to 83 bail! calls and 9 anyhow! calls, and does not account for errors that may be bubbled-up from internal or third-party crates using ?. This is most probably too many instances to create unique variants for all of them, but if we take a closer look at some of these error instances, it becomes clear that we can probably bundle many of them into reusable variants. For example, there are the following bail!s:

src/types/matching.rs
244:                _ => bail!("expected global, but found {}", actual_desc),
248:                _ => bail!("expected table, but found {}", actual_desc),
252:                _ => bail!("expected memory, but found {}", actual_desc),
263:                _ => bail!("expected function, but found {}", actual_desc),
273:                _ => bail!("expected instance, but found {}", actual_desc),
299:                _ => bail!("expected module, but found {}", actual_desc),
310:                _ => bail!("expected global, but found {}", actual.desc()),
314:                _ => bail!("expected table, but found {}", actual.desc()),
318:                _ => bail!("expected memory, but found {}", actual.desc()),
322:                _ => bail!("expected func, but found {}", actual.desc()),
326:                _ => bail!("expected instance, but found {}", actual.desc()),
330:                _ => bail!("expected module, but found {}", actual.desc()),
342:                _ => bail!("expected {}, but found func", entity_desc(expected)),
359:                _ => bail!("expected {}, but found instance", entity_desc(expected)),

These could all be represented by one variant, such as Error::UnexpectedEntityType(String) (name subject to bikeshedding). There are probably other such groupings that can whittle down the number of error variants that would be necessary.

API Commitment

Creating a structured error type would expand the public API of the wasmtime crate, so it may be something that should be done incrementally, or in a future-proof way. One way to do this would be to make the Error enum #[non_exhaustive] so that callers must provide a catch-all when examining it. The initial version of the Error enum could expose some high-impact variants that are obviously useful to be able to inspect, and provide some sort of Other or Unknown case which could store any unclassified errors as an anyhow::Error like how things were done before.

Alternatives

The most obvious alternative is to simply not adopt structured errors, or to not do so at this point in time. This would incur zero maintenance burden and retain the flexibility of being able to change error messages at any time, but would not provide the benefit to programmatic users described above.

peterhuene commented 3 years ago

Hi @nicholastmosher. Thanks very much for raising this issue.

For my better understanding, what sort of errors are you looking to respond to programmatically (not including simply surfacing the error to the user in some fashion) such that the error could be overcome?

If, for example, a module is missing a required export, are you anticipating synthesizing it somehow to get the module to instantiate?

peterhuene commented 3 years ago

I'll mention that with your stated use case, one can simply examine the module's exports prior to instantiation and take the appropriate action without an error from wasmtime.

Generally I break down wasmtime's errors into three categories for the majority of use cases: failure to parse or compile a module, failure to instantiate a module, and failure during code execution.

Failure to parse or compile a module is largely unactionable; bugs in wasmtime notwithstanding, there's not much you can do for an invalid module other than let the user know.

One can inspect a module prior to instantiation to determine compatibility with the host, which isn't in the scope of the wasmtime API itself.

Instantiation failures are generally incompatible imports (see last point) or system resource constraints that wasmtime has limited visibility into itself other that an errno (e.g. failed to allocate virtual memory) That said, there is a class of errors for instantiation that might have value from specific cases: limits placed on instances from within Wasmtime's API.

Lastly, for execution errors, we provide Trap that does offer some context as to why execution failed, since the reasons are easily enumerable.

At any rate, I'm not opposed to changing Wasmtime's error types given a compelling use case.

nicholastmosher commented 3 years ago

Hi @peterhuene sure, let me explain the exact use case I'm working with.

I'm working on Fluvio, a streaming platform written in Rust, and one of the features we support is the ability to use WebAssembly modules to perform custom inline computation on streaming data. This inline computation looks a lot like the higher-order iterator patterns seen in functional programming, e.g. filter, map, fold, and users fill in the implementation of each of these using a WebAssembly module that matches the signature expected in the higher-order function.

To get a little bit more concrete on the interface of an individual WebAssembly module, a filter written by a user will essentially result in the following low-level Rust function being compiled as WASM:

#[no_mangle]
extern "C" fn filter(ptr: i32, len: i32) -> i32 { ... }

This code is generated by a procedural macro in the user's code, and is not written by the users themselves, so the user might write something high-level like this:

#[smartstream(filter)]
fn filter(record: &Record) -> Result<bool> { ... }

However, if the user forgets the procedural macro to generate the low-level code exported at the WASM boundary, the Rust still compiles just fine, but it causes an error on the Streaming Processing Unit server when it tries to instantiate the WASM module and cannot find the function export called filter. We would like to be able to catch a "missing function export" error with confidence, rather than relying on parsing a stringified anyhow::Error. I.e., it would be nice to be able to do something like this:

let filter_fn_result: Result<TypedFunc<(i32, i32), i32>, _> = instance.get_typed_func(&mut base.store, "filter");
let filter_fn = match filter_fn_result {
    Ok(filter_fn) => filter_fn,
    Err(wasmtime::Error::MissingExport(name)) => // Remind the user to add `#[smartstream]` attribute
    Err(other) => // generic error handling
};

One can inspect a module prior to instantiation to determine compatibility with the host, which isn't in the scope of the wasmtime API itself.

This sounds like it would probably be the best bet with our use-case, I'll have to explore more of the API to see how to inspect a module before instantiation. I opened this issue mostly because I was sort of surprised at the use of anyhow in a library and figured this may be a good API improvement that might help a number of use-cases.

peterhuene commented 3 years ago

Module.exports would likely work for your use case.

I agree that surfacing errors structurally makes a lot of sense for libraries and that this is definitely an area that Wasmtime can improve upon.

I am simply wary of introducing such a breaking change at this point without a great reason to do so.

peterhuene commented 3 years ago

I forgot to mention there's also Module.get_export to quickly get at the type for a particular export.

nicholastmosher commented 3 years ago

Thanks @peterhuene, I think exports and get_export will do exactly what I need!

alexcrichton commented 3 years ago

Personally I agree with @peterhuene that @nicholastmosher for your use case inspecting the interface of the Module is the best way to go. Even if we did add an exhaustive listing of errors they're the most likely to change over time since precise errors often get different sorts of context and such like that. In that sense inspecting the module's interface is likely the most stable way to recognize this kind of error.

That being said I do think that Wasmtime should make more use of concrete errors where helpful. I also want to stick to anyhow::Error if we can, but we can always provide public error types for downcasting as we do Trap for wasm traps. That could be used for specific errors and could also be done to recognize mismatches like this if we exported a particular error for this linking error.

bytecodealliance / wasmtime