Clarify rationale - Githubissues

The crate's readme says the crate intends to replace error management in Rust, taking lessons from problems with quick-error and error-chain.

It is intended to replace error management based on std::error::Error with a new system based on lessons learned over the past several years, including those learned from experience with quick-error and error-chain.

Could someone clarify what these problems are with the aforementioned crates, and how this crate aims to solve them?

Here are some comments on that talk. Note that I maintain derive-error-chain and I'm using it to counter some of the points made in the talk that were applied to the error_chain! macro. So those downsides still apply to the error_chain! macro; it's just that there's an alternative syntax that doesn't have those downsides while still using error-chain in the codegen.

Macros have a steep learning curve and give confusing errors for typos. #[derive(Fail)] is simpler to use.

#[derive(ErrorChain)] exists to give a custom derive syntax with the same codegen as error_chain!
error-chain generates two types named ErrorKind and Error and it's not clear what their relationship is and how they should be used.

Perhaps. error-chain does this so that Errors can have causes and backtraces independent of whether the individual variant contains a field marked #[cause] or of type Backtrace. That said, error-chain's handling of cause is IMO wrong for chainable and foreign links, and it does not give you a way to override it. derive-error-chain does give you a way to override it, and I just use custom links in my crates with an explicit cause:
```
#[error_chain(custom)]
#[error_chain(cause = "|err| err")]
ConfigFileNotFound(io::Error)
```
which makes std::error::Error::cause work correctly.
error-chain errors always have a trait object variant.

Is this referring to ChainedError ? While things like kind() and backtrace() are indeed implemented on that trait (to allow cross-type sharing as I said in (2)), the functions are almost always used in a static type context.
error-chain errors always have a backtrace.

Only if the feature is enabled. boats himself mentions this in one of the audience questions later (though he misspeaks that the feature was added "fairly recently" when it's been there since Nov 2016 (v0.6.0)). I assume the intent of this point is that individual error enums can opt in or out of backtraces, but to me that sounds like a bad thing. The final application author should make the decision to enable or disable backtraces.

Later one audience member asks for an environment variable or similar that failure could use to disable backtraces. I'm not sure how it would work since failure's design uses an explicit error field of type Backtrace to store the backtrace in; perhaps by adding a function to the backtrace crate to generate an empty Backtrace object with no frames. The audience member does not appear to have opened an issue to follow up on this.
error-chain "strongly encourages having a single error type for your whole crate." And later "error-chain strongly encourages a unified error type"

The only mention of this in the docs is "Generally, you define one family of error types per crate, though it's also perfectly fine to define error types on a finer-grained basis, such as per module." I don't know whether this means that the docs are "encouraging", let alone "strongly", a crate-level error type. Perhaps it should clarify the second clause that "finer-grained basis" is more appropriate when the errors are sufficiently disparate.

Regardless error-chain has no technical requirement that there be only one crate-level error type, so this is at best a doc bug.
Chainable links give no context about the chained error. links { Io(std::io::Error) } doesn't tell you what operation led to the IO error.

Yes, calling it Io is the wrong choice for something as diverse as std::io::Error. The example I gave in (2) is a better way to write it. But this is a user error, not something wrong with error-chain. Later in one of the failure examples (see (9)) there's an Io(io::Error) field too.
failure::Fail is Send + Sync + 'static so it can be sent across thread boundaries easily.

error-chain has gone back and forth on Sync. This is one place where the decision to have ChainedError and State common to all types hurts error-chain compared to failure's approach, since it's not (?) possible to satisfy both groups of users.
#[derive(Fail) example:
```
#[derive(Debug, Fail)]
#[fail(display = "error code {} occurred", code)]
struct MyError {
    code: i32,
}
```
derive-error-chain does not support deriving on structs, so the smallest example would require making a new enum for it.
```
#[derive(Debug, ErrorChain)]
enum ErrorKind {
    #[error_chain(display = r#"|code| write!(f, "error code {} occurred", code)"#)]
    MyError { code: i32 },
}
```
Wrapping in an enum does look ugly. But it should be possible to implement derive-error-chain on structs though; I just didn't know there was demand for it. I'll look into it.

When arbitrary tts in attributes is stabilized (and syn 0.12 is released with support for parsing arbitrary tts in attributes), the display attribute can be written as:
```
    #[error_chain(display = |code| write!(f, "error code {} occurred", code))]
    MyError { code: i32 },
```
or even:
```
    #[error_chain(display = const("error code {code} occurred"))]
    MyError { code: i32 },
```
(These are only implemented in a branch for now since syn 0.12 is not yet released.)

failure's style is nice and has a bonus that it works on stable, but will cause back-compat issues with derive-error-chain so I cannot add it verbatim. derive-error-chain's method does give more flexibility in that the value of the attribute can be any function expression (see (14)).

Second #[derive(Fail)] example - this time an enum:

#[derive(Debug, Fail)]
enum MyError {
    #[fail(display = "unknown error")]
    UnknownError,

    #[fail(display = "IO Error {}", _0)]
    IoError(io::Error),
}

The equivalent would be:

#[derive(Debug, ErrorChain)]
#[error_chain(error = "MyError")]
enum MyErrorKind {
    #[error_chain(display = r#"|| write!(f, "unknown error")"#)]
    UnknownError,

    #[error_chain(display = r#"|err| write!(f, "IO Error {}", err)"#)]
    IoError(io::Error),
}

and in the future:

    #[error_chain(display = const("unknown error"))]
    UnknownError,
    #[error_chain(display = const("IO Error {0}"))]
    IoError(io::Error),

Talking about how failure::Error is an alternative to Box<std::error::Error>

Nothing to say here. I did notice a mention that being able to return a failure::Error to be able to return arbitrary disparate errors from a single function is a good thing, and I cannot agree. This gives the caller limited ability to handle the underlying error (they have to divine the underlying type from a side channel and downcast it). It's okay that it exists as the equivalent of Box<Error> (but actually downcastable) but it should be avoided.
failure::Fail is no_std-compatible

Like (8), this is another way where having a common trait ChainedError hurts error-chain
Fail is implemented for all std::error::Error, so applications can incorporate existing types.

Nothing to say here. It's the same as having custom links in an error-chain enum.
Audience question - Trying to use two error-chain errors that referenced each other caused a circular From implementation that wouldn't compile. failure does not generate From so does not have this problem.

Custom links don't generate From impls and don't have this problem.
Audience question - error-chain can't localize display error strings. Neither can failure. It's an open problem how errors should support localization.

derive-error-chain supports localizing display error strings, since any function expression can be used in the display attribute. This isn't a limitation of error-chain the library, just the quick_error! based syntax the error_chain! macro uses for display strings.
Audience question - "I used error-chain the way it was meant to be used, by chaining everything to everything else, and compile times went up." This is because chain_err is generic.

I'm not sure if chain_err is the way error-chain "was meant to be used". chain_err is essentially a custom link for arbitrary, unknown errors. Actual links are the better choice when the underlying errors are known types, and also allow you to have more context on them than just the backtrace via display / description.

My 2 cents:

I assume the intent of this point is that individual error enums can opt in or out of backtraces, but to me that sounds like a bad thing. The final application author should make the decision to enable or disable backtraces.

This is a useful thing. Consider this:

enum Error {
    NoSuchElement,
    InternalError(Backtrace),
    ForwardedError(OtherError),
}

When you return NoSuchElement error you don't have to pay the price for backtrace, as it's expected that our imaginary element is just added by the caller. But if something bad happens that we don't know how to handle we return an error with a backtrace. Also, if OtherError contains a backtrace, you often don't need another traceback (and it's not possible to opt out in error-chain as far as I remember)

Note: this is mostly all or nothing thing, i.e. if a library generates a traceback ignoring it or wrapping it into another error doesn't lower the cost (unless it's inlined and optimized out, but we can't always rely on that).

Regardless error-chain has no technical requirement that there be only one crate-level error type, so this is at best a doc bug.

As far as I remember there is a Result type. While you can put it into a submodule, it's still a very big sign that all the library should return this kind of result. I mean importing:

use lib1::moda::Result as ResultA;
use lib1::modb::Result as ResultB;

Technically it works, and technically you don't have to use a Resul type. But it encourages the bad thing.

Chainable links give no context about the chained error [.. snip ..] But this is a user error, not something wrong with error-chain.

This is probably about encouraging .context() pattern which is really very useful. While it can be added to error chain in a way similar to how quick-error done that. Failure's way is more lightweight in terms of lines of code. I've not played with it too much, so I'm not sure it's perfect, but this is the idea.

I did notice a mention that being able to return a failure::Error to be able to return arbitrary disparate errors from a single function is a good thing, and I cannot agree.

There are a lot of places where it makes sense. I use Box<Error> pattern a lot in startup functions (i.e. kinda main), where the only way to handle error is to print an error and exit. And adding traceback here makes a lot of sense.

Another case is in vagga I have a BuiildStep error, which is basically: this build step failed, it could run a command, write a file, or do tons of other stuff. No matter what it was doing we just cleanup the whole container and present the error to the user. And again we can wrap it into a enum Error { Cache, BuildStep(Error) } so that caller can ignore just cache issues.

I assume the intent of this point is that individual error enums can opt in or out of backtraces, but to me that sounds like a bad thing. The final application author should make the decision to enable or disable backtraces.

When you return NoSuchElement error you don't have to pay the price for backtrace, as it's expected that our imaginary element is just added by the caller. But if something bad happens that we don't know how to handle we return an error with a backtrace.

I don't know if it's correct to assume NoSuchElement will never need a backtrace, and then bake this assumption into the type that no consumer can ever modify. If this is in a library, then the author is making this choice for every user of the library, unless the user wraps this into another type / enum variant that has a Backtrace field.

But I understand that it's a waste when compiling a binary where the backtrace isn't elided for a particular variant even though it never gets used. I'm not sure how to get the best of both worlds except by adding a separate backtrace feature for every variant of every error enum that the application author can set as they want, which would be absurd.

Regardless error-chain has no technical requirement that there be only one crate-level error type, so this is at best a doc bug.

As far as I remember there is a Result type. While you can put it into a submodule, it's still a very big sign that all the library should return this kind of result.

It can be renamed just like Error and ErrorKind. The default is to call it Result, but you can have types { Error, ErrorKind, ResultExt, ResultA } in error_chain! or #[error_chain(result = "ResultA")] in #[derive(ErrorChain)]

I did notice a mention that being able to return a failure::Error to be able to return arbitrary disparate errors from a single function is a good thing, and I cannot agree.

There are a lot of places where it makes sense. I use Box<Error> pattern a lot in startup functions (i.e. kinda main), where the only way to handle error is to print an error and exit. And adding traceback here makes a lot of sense.

Right. As I said, it's a fine alternative to Box<Error>. For things that want to ignore the specific error type, it's okay to use Box<Error> or failure::Error (and it's nice that failure::Error is better since it can be downcast if necessary).

The talk mentioned directly returning failure::Error from functions that don't care about the error, and my point is it should be preferred to return a contextual wrapper. That is, rather than fn foo() -> Result<(), failure::Error>, it's better to have fn foo() -> Result<(), FooError> with struct FooError(failure::Error) or enum FooError { UnknownError(failure::Error) }. This way the caller can do something specific for FooErrors (or treat FooError::UnknownError differently from other FooErrors in the enum case) if they need to. They don't have to try to guess that the failure::Error might be a FooError and downcast it.

Chainable links give no context about the chained error [.. snip ..] But this is a user error, not something wrong with error-chain.

This is probably about encouraging .context() pattern which is really very useful. While it can be added to error chain in a way similar to how quick-error done that. Failure's way is more lightweight in terms of lines of code. I've not played with it too much, so I'm not sure it's perfect, but this is the idea.

I don't know what you're referring to about quick-error. failure's Fail::context() / ResultExt::context() are similar to error-chain's Error::chain_err() / ResultExt::chain_err() that already exist. Both sets of API give the caller the ability to create a new error-like thing (a failure::Context for context() and the user's Error type for chain_err()) and hide the original error as its underlying opaque cause (as failure::Error and Box<Error> respectively).

I guess failure's advantage is that the user can call .context() without needing their own Error type to provide ResultExt::chain_err for their crate. It's not hard to make a tiny Error for this purpose, but of course it's not zero:

#[derive(Debug, ErrorChain)]
enum ContextErrorKind<D: Display> { Inner(D) }

result.chain_err(|| ContextErrorKind::Inner(some_context))

(It could be even shorter if #[derive(Debug, ErrorChain)] struct ErrorKind<D: Display>(D); is implemented, like I mentioned in my first comment, but of course still not zero.)

However such a blessed type could just be added to error-chain if there's demand for it, with minimal fuss.

Now if the user does have their own error type, then they might want to retain the original error while still storing a context. Then they would do this with failure:

#[derive(Debug, Fail)]
enum Error<D: Display> {
    #[fail(display = "{}", context)]
    IoError {
        context: D,
        cause: io::Error,
    },
}

and this with derive-error-chain (not with error_chain! since it does not support generic types nor specifying a custom Error::cause()):

#[derive(Debug, ErrorChain)]
enum ErrorKind<D: Display> {
    #[error_chain(custom)]
    #[error_chain(display = "|context, _| context")]
    #[error_chain(cause = "|_, cause| cause")]
    IoError {
        context: D,
        cause: io::Error,
    },
}

So again they're quite even in this regard.

(The idea of reusing any field named cause for the result of Error::cause() instead of specifying #[error_chain(cause = ...)] is appealing. I may do it in derive-error-chain, though I have to balance it with the back-compat concerns.)

@tailhook

As far as I remember there is a Result type. While you can put it into a submodule, it's still a very big sign that all the library should return this kind of result.

Just to add my own two cents, I have always personally used the Result type as a convenience type. When I am working with multiple errors, I just just std::result::Result.

Anyway, I think it's worth mentioning the advantages and the potential disadvantages of this crate in the README.

rust-lang-deprecated / failure

Clarify rationale #66