Stream: git-wasmtime

Topic: wasmtime / PR #5276 wiggle: new error configuration for g...

Wasmtime GitHub notifications bot (Nov 15 2022 at 23:45):

pchickey opened PR #5276 from pch/trappable_error to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Nov 15 2022 at 23:49):

pchickey updated PR #5276 from pch/trappable_error to main.

Wasmtime GitHub notifications bot (Nov 16 2022 at 00:47):

pchickey updated PR #5276 from pch/trappable_error to main.

Wasmtime GitHub notifications bot (Nov 16 2022 at 00:49):

pchickey updated PR #5276 from pch/trappable_error to main.

Wasmtime GitHub notifications bot (Nov 16 2022 at 01:24):

pchickey edited PR #5276 from pch/trappable_error to main:

Largely due to pains in wasi-common and experimentation in wit-bindgen, I have added a slightly different error handling strategy to wiggle.

When the user specifies errno => trappable Error in the macro's optional errors: {...}argument, wiggle will now generate a type named Error to be used in all positions where a function returned a witx errno in the error position.

The generated error type is an opaque struct. Internally, it contains either the errno (which wiggle generates as an enum Errno), or an anyhow::Error for trapping execution. These are constructed using either the generated impl From<Errno> for Error impl, or the Error::trap(t: anyhow::Error) -> Self constructor.

The error can be destructured using Error::downcast(self) -> Result<Errno, anyhow::Error>, inspected using Error::downcast_ref(&self) -> Option<&Errno>, and it can have context added via Error::context(self, s: Into<String>) -> Self. The context is only observable via the Debug and Display impls.

This design came about because the user defined error type wasn't really serving our needs in wasi-common. There, we had essentially 2 choices:

1. the user maintains a thiserror style error enum that contains all of the errno values, plus a Trap(anyhow::Error) variation for trapping execution. They then write From impls for all of the foreign error types that need to be converted into their error enum, e.g. impl From<std::io::Error> for MyErrno. They then implement the (very mechanical) translation to the wiggle generated Errno cases in UserErrorConversion.
2. They use anyhow::Error. To implement the translation to Errno or trap, the user downcasts all of the foreign error types they suspect may be thrown in their implementations.

I used both of these designs in wasi-common, in that chronological order. I found that both had real drawbacks:

1. This error enum is repetitive to define and maintain. Additionally, it doesn't provide any way to keep additional context around on an error, which makes debugging harder.
2. Suffers from safety problems. Since introducing this design in the 2019 wasi-common rewrite, I (unknowingly) introduced a bug where a cap_rand::Error was thrown in the random methods, but I never tried to downcast to it in UserErrorConversion. The end result is that any error in the cap_rand crate would end up trapping execution of the wasm module, rather than returning it an appropriate errno. I don't think I ever would have discovered this bug if I hadn't attempted a refactor back towards design 1.

So, my design here basically uses code generation to generate an acceptable implementation of design 1, while providing the .context("help figuring out what went wrong") method available in design 2. I was reluctant to give up the ergonomics of approach 2, but the design of wasi is evolving to give us lots of different concrete errnos in the preview 2 interfaces, so I wanted to give us as much type safety as possible when managing the complexity of the coming evolution in this crate.

Wasmtime GitHub notifications bot (Nov 16 2022 at 01:32):

pchickey updated PR #5276 from pch/trappable_error to main.

Wasmtime GitHub notifications bot (Nov 16 2022 at 01:49):

pchickey has marked PR #5276 as ready for review.

Wasmtime GitHub notifications bot (Nov 16 2022 at 16:54):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Nov 16 2022 at 16:54):

alexcrichton merged PR #5276.

Last updated: Nov 22 2024 at 16:03 UTC