[Task] Implement a serializable error

[PhilippGackstatter]

Motivation

The upcoming actor exchanges messages over the network, where errors can occur. The current error enums we use are not serializable, and thus cannot be exchanged between actors. Another motivation may be to improve the currently stringly-typed errors in the WebAssembly bindings. In order to pass a value to JavaScript, it needs to be serializable. Errors also need to be exposable to future bindings, which often involves (de)serialization.

Description

Implement an error type that is serializable for over-the-network and bindings usage. WebAssembly bindings are a good case study. We currently use error enums in Rust, but any bindings that use a C FFI as the common interface, such as Wasm, can only use C-style enums. We have talked about this a number of times, and I want to summarize the approaches we've discussed, as it is not entirely clear yet, which is the best.

The general idea is to use an error code approach, and we've discussed variants of it. As an example we consider a part of the account error enum.

pub enum Error {
  #[error("Stronghold snapshot password not found")]
  StrongholdPasswordNotSet,
  #[error("Stronghold error: {0}")]
  StrongholdResult(String),
  #[error(transparent)]
  IoError(#[from] std::io::Error),
  #[error(transparent)]
  StrongholdError(#[from] iota_stronghold::Error),
  ...
}

The simplest approach would be to map each of the variants to some error code. For the variants that contain associated data, information will be lost (or preserved in stringified form). On the receiver side, the error cannot be reconstructed, since not all information is available. Furthermore, since no enum is defined, the receiver only has the bare number but has a hard time attaching a meaning to it.

Thus, to build on this approach, we define roughly this Rust code:

pub struct Error {
  code: ErrorCode,
  stringified: String,
}

pub enum ErrorCode {
  InternalError = 0,
  StrongholdPasswordNotSet = 1,
  StrongholdResult = 2,
  ...
}

To convert the example variants into this Error, we might do:

• the StrongholdPasswordNotSet variant is easily translated as some error code.
• the StrongholdResult has an associated String. We can translate the variant itself to some number, and additionally transmit the stringified error.
• The IoError is a fatal error from the other side, and the caller cannot do much about it, similar to a 5XX HTTP status. These errors can be mapped to a catch-all InternalError (or similar) and also stringified to provide more detailed human-readable information. Some errors also should not be exposed, such as StrongholdMutexPoisoned (not represented in the example above), which can similarly be mapped to the catch-all variant.
• The StrongholdError can either be mapped to a single StrongholdError error code, or each of its variants can be mapped to error codes. The former approach means the caller has less information available, i.e. cannot programmatically match on specific variants. It is at least mitigated by the stringified version in terms of human readability. The latter approach includes a lot more maintenance burden. One other idea was to use two integers. E.g. to map StrongholdError, we store its error code in the first integer, and use the second integer to store error code of the inner variant. This has very high maintenance costs. To convert two integers back into a nested enum, we would need to copy the entire first two levels of nesting of the Error enum introduced above into new serializable enums.

There is perhaps more investigation needed to find the optimal approach. An implementation with unions might be viable, although given the unsafety they introduce and the conversion that would also be necessary, similar to the previous approach, makes this less desirable.

Any feedback on this write-up is welcome!

Resources

• meeting minutes from identity standup, 2021-07-14

To-do list

Create a task-specific to-do list . Please link PRs that match the To-do list item behind the item after it has been submitted.

• Find an error implementation that is maintainable, is as explicit as necessary, and memory-safe
• Implement the error system and provide conversions from the identity_account::error::Error and perhaps identity_iota::error::Error.
• Ensure compatibility with WebAssembly bindings as early as possible, in particular conversion to exceptions

Change checklist

Add an x to the boxes that are relevant to your changes, and delete any items that are not.

• The feature or fix is implemented in Rust and across all bindings whereas possible.
• The feature or fix has sufficient testing coverage
• All tests and examples build and run locally as expected
• Every piece of code has been document according to the documentation guidelines.
• If conceptual documentation (mdbook) and examples highlighting the feature exist, they are properly updated.
• If the feature is not currently documented, a documentation task Issue has been opened to address this.

[cycraig]

There are two problems presented here:

1. (De)serializable errors for the actor handlers
2. Error handling for bindings (specifically WebAssembly/wasm-bindgen)

Serializable errors are currently the most important as they prevent actor handlers implementing branching behaviour based on error types.

To recap, the example Error below cannot be serialized as any nested errors not defined in the identity codebase (e.g. std::io::Error) cannot be (de)serialized easily. That is, we cannot derive nor manually implement serde::Serialize and serde::Deserialize for those errors without losing at least some context (which generally prevents deserialization back to the same underlying error type).

pub enum Error {
  #[error("Stronghold snapshot password not found")]
  StrongholdPasswordNotSet,
  #[error("Stronghold error: {0}")]
  StrongholdResult(String),
  #[error(transparent)]
  IoError(#[from] std::io::Error),
  #[error(transparent)]
  StrongholdError(#[from] iota_stronghold::Error),
  ...
}

Aside: we also want to avoid std::io::Error in the actor in general for no_std compatibility.

---

Considering just (de)serialization, we have several options.

Option 1: serde_error

One way to serialize errors is the serde_error crate, which essentially converts the error to a string and boxes the source for nested errors.

https://github.com/Ekleog/serde-error/blob/master/src/lib.rs:

#[derive(serde::Deserialize, serde::Serialize)]
pub struct Error {
    description: String,
    source: Option<Box<Error>>,
}

One could easily wrap all errors in the actor with serde_error::Error for (de)serialization and sending over the wire. However matching on the receiving side would require downcasting back to the enum type, which is clunky and I suspect is not something easily achieved across FFI boundaries. The serde_error crate also relies on std::error::Error and thus is not no_std compatible. For these reasons, I do not consider it to be a viable option in our case.

There is also the anomaly crate which has out-of-the-box support for serde serialization of errors. However, it requires that our error types already implement serde::Serialize to use, so it does not solve the problem of unserializable nested errors.

---

Option 2: Rust enums

We could convert the inner errors to strings to create a serializable Rust enum.

#[derive(serde::Serialize, serde::Deserialize)]
pub enum ActorError {
    // specific errors that the actor might want to handle conditionally
    StrongholdPasswordNotSet,
    StrongholdResult(String),
    ...

     // non-specific errors
    #[cfg(feature = "account")]
    IdentityAccount(String), // any other error from identity-account
    IdentityIota(String), // any other error from identity-iota
    Handler(String), 

    // unknown error due receiving an unrecognised error type or one that was not serialisable
    Unknown(String),
}

The advantage of this approach is that it's closer to idiomatic Rust errors, but loses the context of any nested errors.

The Unknown variant is for extensibility and interoperability between different actor versions where one side has defined more or fewer enum variants, any unknown variant can be manually caught and treated as Unknown. We might be able to avoid the Unknown variant in Rust code by using the non_exhaustive attribute, but deserializing unrecognised error enum types would still need to be handled manually.

---

Option 3: C-style enums

This is the approach suggested in the issue, whereby we wrap errors in a struct with a code mapping to a flat C-style enum and a stringified representation or description of the error.

#[derive(serde::Serialize, serde::Deserialize)]
pub struct ActorError {
  code: ActorErrorCode,
  description: String,
}

#[derive(serde::Serialize, serde::Deserialize)]
pub enum ActorErrorCode {
  InternalError = 0,
  StrongholdPasswordNotSet = 1,
  StrongholdResult = 2,
  ...
}

We can still optionally implement std::error::Error for the struct and use it in normal code.

#[cfg(feature = "std")]
impl std::error::Error for ActorError {
...

The difference would be that pattern-matching has to be performed on the code rather than the error itself (ignore the direct field access for now):

match error.code {
  ActorErrorCode::StrongholdPasswordNotSet => {
    println!("{}", error.description);
    ...

The advantage is that such an error may be exported and used directly in any bindings that can only handle C-style enums such as wasm-bindgen . The disadvantages are similar to Option 2 where the inner error types are erased with the additional drawback of slightly less idiomatic usage w.r.t. pattern matching.

---

Error handling for bindings

When considering the impact/necessity of defining the errors to be compatible with language bindings, it should be noted that wasm-bindgen and C seem to be outliers in requiring C-style enums. Notably, Uniffi-rs claims to convert Rust error enums into throwable exceptions directly for its supported languages (Kotlin, Swift et. al.). Python bindings with PyO3 just requires us to implement From<CustomError> for PyErr for proper exceptions, so it too does not really benefit from C-style enums.

There are other issues with C bindings where even strings need to be converted to compatible types manually, and error handling in C (following an idiomatic approach similar to libgit2) would require a lot of manual effort regardless of whether we export C-style enum errors or not. We could theoretically use (unions)[https://doc.rust-lang.org/reference/items/unions.html] in the C bindings as suggested but definitely not in the core libraries in my opinion, due to the use of unsafe.

This leaves us to consider the wasm-bindgen bindings. Note that errors are usually converted to some opaque JsValue in wasm-bindgen (including in the current bindings in identity.rs), which may be thrown as a regular exception in JavaScript. The benefit of C-style enums (option 3) there would be the ability to (try) deserialise the JsValue back into an ActorError in the JavaScript code and then match on the code for branching behaviour / recoverable errors rather than just throwing an opaque exception string. There have been quite a few discussions around a more dedicated alternative to JsValue such as JsError (wasm-bindgen issue) but nothing has materialised as of yet. If in the majority of cases users of the wasm bindings simply throw an exception, there might be little benefit.

My main concern is that developers using the Rust version of identity.rs should not suffer due to any changes introduced for bindings. That is, we need to retain the ability of pattern matching nested errors without type erasure where possible, such as in identity-account and identity-iota.

Conclusions

With the above in mind, my preference would be to do all of the following:

• Retain our current (non-serializable) errors in existing projects (e.g. identity-account, identity-iota)
• Create serializable errors for actor handlers in identity-actor
  • e.g. create a serializable AccountError which wraps the identity-account error, which may be nested (option 2) or stringified (option 3) in another ActorError if needed (I prefer option 2).
• Create dedicated error wrappers in each of the bindings as-needed (so option 3 for wasm-bindgen). This is more maintenance but it preserves nested error pattern-matching for Rust developers. Note that wrappers are needed regardless for some languages e.g. C bindings.

Note that if we go with C-style enum errors for the actor those may be re-used for bindings as they would wrap the identity-account and identity-iota errors. This would still require a refactor of the wasm bindings error handling as they currently use JsValue, or provide a function or example on how to deserialise a JsValue into e.g. an AccountError.

Finally, it was noted in discussions that developers are able to create their own error types for custom actor handlers, as long as they are (de)serializable (https://github.com/iotaledger/identity.rs/blob/feat/identity-actor/identity-actor/src/traits.rs#L23):

pub trait ActorRequest: Debug + Serialize + DeserializeOwned + 'static {
  type Response: Debug + Serialize + DeserializeOwned + 'static;
  ...

Where the Response can be defined as a result with a serializable error, e.g. Response = Result<IotaDocument, AccountError>. This does not change much other than the fact that we won't have a centralised ActorError expected to be used by all actor handlers.