Consider replacing failure with std::error

[matklad]

I've noticed that chalk used failure for error handing, but I wonder if this is really necessary?

It seems like today, failure actually doesn't buy much over std::error (only backtraces) and that it hasn't become the goto error handing solution for rust, as it once was hoped.

[shepmaster]

I'll recommend the SNAFU crate to reduce some of the boilerplate of implementing std::error::Error. I've had positive interactions with the crate author in the past 😉.

[matklad]

Yeah, I've also heard good things about SNAFU, but I wonder if error-helpers make sense in the context of chalk at all?

Like, chalk doesn't do IO, so all errors are "the input program is wrong", they are not "something randomly crashed 1000 frames deep in the guts of a framework".

[nikomatsakis]

@shepmaster is the idea of SNAFU that derive(Snafu) generates the selector names that sit alongside the enum?

[nikomatsakis]

Must be, I guess

[shepmaster]

@matklad here are my thoughts:

they are not "something randomly crashed 1000 frames deep in the guts of a framework".

"frames" and "stacktraces" should be the method of last resort for figuring out what went wrong. Instead, I like to think of a "semantic" stack trace. From other comments in the SNAFU repo:

I roughly equate each error type to a semantic level in a backtrace. When something wrong happens in a program, I think it's a useful exercise to explain as a human what it was:

1. I couldn't upload the image I to the server S because
2. I couldn't load the username and password from the config file C because
3. I couldn't find the file

#[derive(Debug, Snafu)]
enum Error {
    UploadFile { source: config::Error, image: path::PathBuf, server: String },
    // ... 
}

mod config {
    #[derive(Debug, Snafu)]
    enum Error {
        LoadCredentials { source: io::Error, path: path::PathBuf }, 
        // ...
    }
}

I don't know enough about chalk to say for certain, but I definitely have the belief that while it might not have 1000 stack frames, it does have numerous tricky cases that it wants to report, ideally reporting in a "nice" way.

Using the same kind of idea, poorly applied to chalk's domain:

• Unable to find a method foo for the type Bar. The following methods almost matched:
  • The method foo from the trait Quux does not apply
  • The trait bounds [...] were not met.

#[derive(Debug, Snafu)]
enum Error {
    UnableToFindMethod { name: String, type: String, attempted: Vec<TraitNotApplicableError> },
    // ...
}

#[derive(Debug, Snafu)]
enum TraitNotApplicableError {
    /// The trait is not implemented for this type
    NotImplemented,
    /// The trait is implemented for this type, but the predicates are not true
    PredicatesFailed { predicate: WhereClause },
    /// The trait requires that the trait implements the trait
    CircularTraitResolution,
}

chalk doesn't do IO

I hope that you don't think that SNAFU is only good for programs that are IO-based. (if you do, please tell me because I need to adjust my messaging to avoid that). The primary reason I use std::io::Error is to highlight that a foo::Error error in one context is different from a foo:Error in another context.

There's nothing unique about IO in this view.

all errors are "the input program is wrong",

If this is true, then I'd expect that you could just return Result<Foo, ()> — if you get an Err, you know the problem is that the input program was wrong. I doubt that's what you mean! Instead, think of SNAFU as allowing you to express what about the program was wrong.

[shepmaster]

@nikomatsakis yep, that's correct. It's the single most contentious design decision in SNAFU. I hope to switch to a procedural macro someday (when we bump our minimum Rust version, perhaps?) as many people have expressed concern at a derive(...) doing anything except implementing a trait.

[matklad]

@shepmaster let me try to rephrase why I feel that usual error handling mechanisms of Rust are not really helpful for chalk or other compilery things.

snafu, failure, error_chain, in my mind, are geared towards handling "exceptional" situations. That is, cases, where you have a wide variety of not really related errors, which should not be handled until the application/request boundary. Here, chaining, ability to merge cases across unrelated libraries via enum or dynamic dispatch and support for backtraces (semantic or not) is really useful, so that you can make sense of the error which originated in the guts of the application, at the outer layer of the app.

In compilers, errors are not exceptional, you often use (T, Vec<E>) rater than Result<T, E> to be able to emit multiple diagnostics and soldier on, errors have more explicit presentation than just fmt::Display, and, more generally, don't make use of std::error::Error as they are handled on the spot (by emiting diagostics, fixes, etc), and not handled to upstream libraries, and there are typically no downstream libraries that emit their errors.

So, I don't feel like error helpers buy much for chalk's use-case, but they certainly add to the number of deps and compile time.