Should we recommend middle-layer libraries add context to errors?

[withoutboats]

For "leaf" libraries and "root" applications the story from failure is pretty clear - leaf libraries should define new types that implement Fail, and applications should use the Error type (unless they are no_std applications).

But what about the in-between space? What if you depend on a bunch of libraries with their own error types, and introduce a few of your own as well?

There are a few options I know about:

1. Just return Error. In many cases, this is very convenient. The downsides are that you require the Error type (making it inappropriate to call these functions when failure could be in the hot path) and that users get no additional context about what the error means in the context of your library - libfoobar threw an io::Error, what does that mean?
2. Wrap context around the underlying error. Importantly, this contextual information can have a many-to-many relationship: an underlying error could have many different semantic meanings about "what has gone wrong," whereas many different kinds of underlying errors could mean the same thing at this level of semantics. This requires manual application of context when the error occurrs (and can't be supported by a standard ? conversion), and when users are trying to downcast they now have to deal with all of this additional context information.
3. Define a new error type with From conversions. This is what error-chain heavily encourages. This ends up being the worst of both worlds IMO: you get a bulky chain of errors, and each new layer tells you nothing. A FooBarError::Io(io::Error) doesn't tell you anything that just a libfoobar function throwing an io::Error and casting it to the general Error wrapper wouldn't also have told you. It gives no context.

As an example of how to use Context<D> from failure to do the contextual form:

#[derive(Debug, Fail)]
#[fail(display = "{}", inner)]
pub struct Error {
    inner: Context<ErrorKind>,
}

#[derive(Debug, Fail)]
enum ErrorKind {
   #[fail(display = "invalid Cargo.toml")]
    InvalidCargoToml,
    #[fail(display = "corrupted registry index for registry {}", _0)]
    InvalidRegistryIndex(String),
    #[fail(display = "network error")]
    NetworkError,
}

You can now use the .context() method to add an ErrorKind to an underlying error like an io::Error, a parsing Error, or an error in git.

I think 1 and 2 are both broadly applicable depending on the use case, whereas 3 (what error-chain encourages today) is rarely a good idea (basically, only if you can't afford to do the allocation in Error). But when do we recommend 1 over 2, or vice versa? How do users know whether they should introduce a contextual construct?

[durka]

I might just be restating the question, but when do you use Context vs creating a new error type or variant? The book implies you should use Context when you're bubbling up a library's error and you have more information to add. But here you're suggesting a library would use Context<Foo> as its own error type. But you still have to add variants to Foo for any dependencies' errors. Why not include the context in the error variants? For example, when I use error-chain I usually do this:

error_chain! {
    errors {
        Io(op: &'static str, path: PathBuf)
    }
}

let out = File::create(&output_file).chain_err(|| Io("create", output_file.clone()))?;

So the context is in the variant and the original io::Error is in the cause/backtrace. What should I do with failure instead?

[durka]

I think what you're recommending is I should do (assuming your Error and ErrorKind definitions):

let out = File::open("Cargo.toml").context(ErrorKind::InvalidCargoToml)?;

Is that right? It seems pretty similar to propagating an error-chain error, really. Except you could end up with a chain of contexts instead of a chain of causes.

[withoutboats]

Full code sample for a "context adding error," that guidance will recommend for complex, intermediate libraries that want to provide the most robust error information:

use std::fmt::{self, Debug, Display};
use failure::{Fail, Context};

pub struct MyError {
    inner: Context<MyErrorKind>,
}

impl MyError {
    fn new(kind: MyErrorKind) -> MyError {
        MyError { inner: Context::new(kind) }
    }
    
    pub fn kind(&self) -> MyErrorKind {
        *self.inner.get_context()
    }
}

impl Fail for MyError {
    fn cause(&self) Option<&Fail> {
        self.inner.cause()
    }
    
    fn backtrace(&self) -> Option<&Backtrace> {
        self.inner.backtrace()
    }
}

impl Display for MyError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        Display::fmt(&self.inner, f)
    }
}

impl Debug for MyError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        Debug::fmt(&self.inner, f)
    }
}

impl From<Context<MyErrorKind>> for MyError {
    fn from(inner: Context<MyErrorKind>) -> MyError {
        MyError { inner }
    }
}

#[derive(Copy, Debug, PartialEq, Eq, Fail)]
enum MyErrorKind {
    // C-style enum
}

You can throw these errors with the .context adapter on ResultExt:

use failure::ResultExt;

do_some_io().context(MyErrorKind::WhatThisErrorMeans)?

It allows a many-to-many relationship between the errors at the level of your library (enumerated with "MyErrorKind") and the underlying error types of errors your dependencies throw. Users can access both with .kind() and .cause().

Probably we will eventually want a way to generate this code. I can think of a few different syntaxes:

#[derive(Fail)]
#[fail(context_wrapper)]
struct MyError {
     inner: Context<MyErrorKind>,
}

context_wrapper!(MyError <= MyErrorKind);

Going to punt on codegen for this pattern for 0.1.

[withoutboats]

Added guidance about this pattern at https://boats.gitlab.io/failure/error-errorkind.html

[SuperFluffy]

I am trying to implement an ErrorKind using the example in https://boats.gitlab.io/failure/error-errorkind.html, and am not completely sure how this helps me wrap IO errors.

My errors look like this (it's actually a bit more complicated, because LinuxI2CError would come from an associated type of a trait, but the below is fine for my question):

use i2cdev::linux::LinuxI2CError;

#[derive(Clone, Copy, Fail, Debug)]
#[fail(display = "My error occured")]
pub struct OutputLevelError {};

#[derive(Debug, Fail)]
enum MyCombinedError {
    I2CError(LinuxI2CError),
    MyError,
}

Now underneath are IO and Nix errors - and both have a myriad of different ways in how they can occur.

What is the suggested solution in how I attack those? The way it currently stands I feel like I would need to match on the various errors kinds IO and Nix and provide extra context for each single one. But that appears silly.

[withoutboats]

The idea of this pattern was to insert information about what failed at the level of abstraction you're writing - this shouldn't depend on the underlying cause of the LinuxI2CError.

As a classic example: you have a function which reads some manifest. You wrap all of the errors in the context which ends up printing "could not process manifest". When an error occurred, users end up getting both the underlying IO or parsing error as well as your additional information that this error occurred in the context of trying to process the manifest file.

But no matter what the underlying error was, at your level of abstraction the action which failed is the same: you couldn't process the manifest.

Its possible that you need different context depending on variants of the underlying error, but this pattern isn't really trying to solve that. I'm not sure if you actually do, though - don't the variants convey sufficient information about the errors at their level of abstraction?

[SuperFluffy]

I think my question stems from me not completely understanding how the library and this pattern work. This statement of yours makes my misunderstanding clear to me:

When an error occurred, users end up getting both the underlying IO or parsing error as well as your additional information that this error occurred in the context of trying to process the manifest file.

I think I am starting to understand how the library works, and that ResultExts context method is responsible for providing that Context that allows me wrap those IO or Nix errors and integrate them.

I had the impression that I would lose those errors and replace them by less information.

[withoutboats]

To be clear, the Display impl ("{}") of the Context type only shows the context message, but the Debug impl ("{:?}") shows the whole thing.

So you have a choice when displaying the error to:

• Only show the context message with Display
• Show all the messages with Debug
• Iterate through the causes and print them as separate messages using the causes iterator.

Depending on how the error is being printed, any of these may be appropriate.