[Task] Improve errors

[olivereanderson]

Description

Improve the errors in this library by (incrementally) implementing @cycraig's suggestion copied here:

1. Reduce the number of top-level errors by grouping related variants. This may also be achieved by defining sub-error enums for specific cases (or modules) like with DIDError. (Edit: for future reference, the decision of when to define sub-error enums or inline the error message is likely based on how often that message is repeated and how many distinct variants there are. One instance => inline the message, multiple tends towards sub-error enums).

E.g.

  #[error("Invalid Document - Missing Message Id")]
  InvalidDocumentMessageId,
  #[error("Invalid Document - Signing Verification Method Type Not Supported")]
  InvalidDocumentSigningMethodType,

can become:

  #[error("invalid DID Document - {0}")]
  InvalidDocument(&'static str), // or String if dynamic values are required.

return Err(Error::InvalidDocument("missing message ID"));
return Err(Error::InvalidDocument("signing verification method type not supported"));

2. Improve error messages to include more information where appropriate, rather than generic, obscure messages.

• See: https://www.morling.dev/blog/whats-in-a-good-error-message/

E.g.

  #[error("Invalid Root Document")]
  InvalidRootDocument,

needs more context:

  #[error("invalid root document - {0}")]
  InvalidRootDocument(&'static str),

    // The previous message id must be null.
    if !document.metadata.previous_message_id.is_null() {
      return Err(Error::InvalidRootDocument("not first in chain due to non-null previous_message_id"));
    }
	
    // Validate the hash of the public key matches the DID tag.
    let signature: &Signature = document.try_signature()?;
    let method: &IotaVerificationMethod = document.try_resolve_method(signature)?;
    let public: PublicKey = method.key_data().try_decode()?.into();
	if document.id().tag() != IotaDID::encode_key(public.as_ref()) {
      return Err(Error::InvalidRootDocument("signature verification method does not match DID tag"));
    }

3. Change errors to include a source where appropriate, to enable error chaining/context for application error handlers. This is supported by the discussions in the following Rust Error Handling Project issues:

• https://togithub.com/rust-lang/project-error-handling/issues/44
• https://togithub.com/rust-lang/project-error-handling/issues/27

Specifically this comment which defines the project's official guidance as:

Return source errors via Error::source unless the source error's message is included in your own error message in Display.

4. Replace blanket #[from] implementations of external errors with errors for specific cases as mentioned in Strategy 4. NOTE: this may actually increase the number of variants, so it does not have to be a hard rule.

EDIT 27-04-2022: Also avoid directly wrapping external errors from pre-1.0 crates unless said crate is known to be much closer to a stable release (version >= 1.0) than this library. More specifically do NOT do this:

pub enum Error {
    SomethingWentWrong(external_crate::Error)
   // other variants
}

when external_crate is pre 1.0.

6. Mark exposed error enums as #[non_exhaustive] to prevent technical breaking changes from introducing new variants, similar to DIDError.

7. Consider using anyhow/eyre to construct a report including context for error messages in the Wasm bindings. (Edit: to be clear this is because while Rust can report the context by using anyhow/eyre, Wasm needs an alternative.)

Suggestions 1-5 can be done incrementally and do not require a major all-at-once refactor, so we can easily explore whether or not they work in a single crate/module independently or combined. Suggestion 6 is confined to the Wasm bindings where we have more leeway.

Motivation

In this discussion we highlighted that our current error approach exposes many details which both leads to problems in terms of stability as well as a high cognitive burden on the user. After discussing ways in which we can improve our approach to error handling in the previously mentioned discussion and in #636 we decided on applying this approach incrementally and re-evaluating whether something more needs to be done to improve the errors in the JS bindings once we are done.

How to contribute

Submit a PR that improves an error in this library following one or more of points 1- 6 from this issue's description. The PR does not need to be substantial, it can be as simple as grouping two related variants in a crate's error (see the example following point 1.).

[olivereanderson]

Here is an interesting question related to implementing the guidelines in the project:

Question: The guidelines state the following:

Return source errors via Error::source unless the source error's message is included in your own error message in Display.

But are there any particular situations where one way is preferred over the other?

Answer: As a rule of thumb one should prefer to return the source error via Error::source. The reason for this is that otherwise one can end up with a situation like the following:

Error_1 (root cause) -> Error_2 -> Error_3 (top error)
if Error_3 includes the error message of Error_2 in Display then it follows from the guidelines that we must have Error_3::source() = None, but if Error_2::source() = Error_1 then Error_2's Display cannot contain the error message of Error_1 so the error message of Error_1 (the root cause) will at best be rather inconvenient to obtain or at worst there will be no way to obtain it depending on the representation of the errors involved.

Note that the problem described above can only exist when the two approaches are interleaved so one could also recommend never returning a source and always including the error message of the cause in an error's Display, but that is arguably less convenient for users who for instance want to format error chains themselves (using Error::chain when it stabilises).