comit-network / waves

Workspace for project waves - bringing DeFi to Liquid.
MIT License
15 stars 6 forks source link

On error handling inside the Rust wallet #232

Closed thomaseizinger closed 3 years ago

thomaseizinger commented 3 years ago

Current situation

At the moment, error handling inside the Rust wallet is a bit of a mess.

  1. There are error enums but they all just wrap anyhow. That is pretty pointless because one could just tuck another message onto an anyhow::Error using .context and save yourself the hassle of typing out multiple enums.

If dedicated error enums are favored for some reason, they should be used in an idiomatic way. At the moment, the error message of the source error is manually included in the display strings using {0}. Instead, we should we marking the inner error as #[source] and the inner error message should not be included in the variant's message.

  1. Baru will expose better messages with https://github.com/comit-network/baru/issues/61. So that should improve some of the error messages.

Recommendation

At the moment, error messages are just piped through to the TypeScript code as a chain of causes (Printing an anyhow::Error with {:#} prints the entire chain of causes).

If that situation remains the same, I would recommend to simply make extensive use of anyhow's .context to enrich the error stacktrace with messages. This should result in basically no error handling boilerplate and hence be very ergonomic to use.

The only downside of this approach is that the entire error stacktrace is piped through to the frontend and (I think?) displayed to the user which is not particularly nice. To change that, we need to add an explicit conversion step from individual causes to nice error messages.

My recommendation here would to be to gradually adopt the above pure-anyhow solution in the following way:

  1. Treat the entirety of content-script, background-script, etc as dumb pipes and assume any error received on the JavaScript side can just be displayed to the user because it is nicely formatted.
  2. To achieve a nice formatting of errors, downcast the anyhow::Error at the top-level of the wasm-bindgen functions to the specific source that you are interested in and map it into the error message you would like the user to see.
  3. To be able to .downcast an anyhow::Error into a concrete one, the type needs to be nameable. Using .context with a String doesn't allow this. Hence, to be able to .downcast an anyhow::Error to a specific cause, you will need to introduce a dedicated error type for this. Most importantly, you don't need to give up the use of .context for this but instead you can just pass your error type into .context:
#[derive(thiserror::Error)]
#[error("Failed to do something")]
struct MyError;

// .... somewhere in your code

let foo = do_something().context(MyError)?;

// .... in the `wasm-bindgen function

let result = match wallet::do_something() {
    Ok(foo) => // ...,
    Err(anyhow) => {
        let maybe_my_error = anyhow.downcast::<MyError>();

        // return specific error message based on `MyError`
    }
}