on error handling

[BurntSushi]

I realize opinions might run a bit strong here, but I'd like to make a case against the use of error-chain (or failure) for error handling in the cookbook examples. I think the error handling mechanisms provided by the standard library should be used instead.

Some background: I've been using Rust for a long time (since before 1.0), so the cookbook isn't actually something I've ever read in close detail before. I've also never used error-chain before (which I think probably reveals some bias in my argument here, FWIW).

I have an experience report from a colleague of mine that is in the process of learning Rust. They were using this cookbook as a way of reading example code. One of the things they did in their own code was copy the use of Result<()>. But their code wouldn't compile and they couldn't figure out why. When they asked me, I said, "oh, you're probably missing a type Result<T> = ...; somewhere in your code." But then they pointed me to the example in which they saw Result<()>, which does not have any such type alias.

It took me at least a few minutes of perplexed digging before I was actually able to figure out how that code was compiling. It turns out that the error_chain! macro is defining a Result type alias for you. Personally, I think this makes learning from these examples much harder than necessary. Some might even argue that the Result type alias itself is problematic, but even aside from that argument, I think having it implicitly defined and brought into scope is really quite surprising, especially for beginners that might be trying to learn Rust through this cookbook.

Of course, this is just one experience report. In any case, what are the alternatives? Here are some that I can think of:

• Keep error-chain, but don't use the Result type alias. (What does one use instead that doesn't suffer from the same implicitness fault?)
• Switch to using failure.
• Switch to using only the facilities provided by the standard library.
• Remove proper error handling altogether and use unwrap/panic.

I don't think we should remove proper error handling. I think a cookbook that shows how to do error propagation is much more valuable than a cookbook that doesn't. And I think such examples are incredibly useful for beginners.

I don't think we should use failure because its development is a bit in flux, despite it being used in a fair number of crates. In particular, there is a merged RFC to fix the standard library's Error trait, and this will likely have an impact on failure's API. So I think there is a collective "let's wait and see" on that front. However, if and when failure's future becomes solidified, it might be a good idea to transition to using it.

I also don't think we should use error-chain either, even if we were able to fix the implicitness problem in the examples today. Of course, fixing the implicitness would be something that I'd see as an improvement! For the most part, I perceive error-chain as something that the ecosystem is probably not going to adopt. I think the direction is likely headed more towards failure, or perhaps, even std itself.

Ultimately, my argument concludes here that we should just use what the standard library provides. In particular, if I were to write the aforementioned example:

#[macro_use]
extern crate error_chain;

use std::fs::File;
use std::io::{Write, BufReader, BufRead};

error_chain! {
    foreign_links {
        Io(std::io::Error);
    }
}

fn run() -> Result<()> {
    let path = "lines.txt";

    let mut output = File::create(path)?;
    write!(output, "Rust\n💖\nFun")?;

    let input = File::open(path)?;
    let buffered = BufReader::new(input);

    for line in buffered.lines() {
        println!("{}", line?);
    }

    Ok(())
}

quick_main!(run);

then I would write it like this:

use std::fs::File;
use std::io::{self, Write, BufReader, BufRead};
use std::process;

fn run() -> io::Result<()> {
    let path = "lines.txt";

    let mut output = File::create(path)?;
    write!(output, "Rust\n💖\nFun")?;

    let input = File::open(path)?;
    let buffered = BufReader::new(input);

    for line in buffered.lines() {
        println!("{}", line?);
    }

    Ok(())
}

fn main() {
    if let Err(err) = run() {
        eprintln!("{}", err);
        process::exit(1);
    }
}

If an example requires dealing with multiple errors, then I'd either write out a new error type, possibly with appropriate From impls, or just use Box<Error>. Namely, Box<Error> is used in command line tools effectively as a means of bubbling an error up to main. The downside to using Box<Error> is that it's probably not something one wants to use in a library.

I also believe that the examples should not return Result directly from main, as suggested in #433. I explained my position on that front in more detail here. With that said, I personally feel less strongly on this point than I do about using error_chain or failure.

Finally, I certainly don't intend to say that error-chain (or failure) should be expunged from the cookbook entirely. I just think they shouldn't be the go-to error handling mechanism for every single example.

Thanks for your consideration! And thanks so much for all your hard work on this wonderful resource. I'd like to end on a positive note: my colleague reports that the cookbook has been a wonderful resource for learning Rust. In particular, he found that the cookbook---above other docs---made it much easier to learn how to actually start writing a full program. That's an awesome compliment!

[budziq]

Let me start that I love your feedback and hope to make it into something actionable!

I agree that the current error handling situation is less than ideal, mostly due to error-chain being both too "magical" and heavyweight. Also I second that if the whole error handling can be performed with builtin / imported Result / Error types the error-chain usage should ideally be removed.

I can't say that I love the idea of writing explicit error handling in main

fn main() {
    if let Err(err) = run() {
        eprintln!("{}", err);
        process::exit(1);
    }
}

and presenting it on nearly every example along with hand crafted error types. This would get rather verbose and would lead the examples to be quite noisy (error boilerplate vs actual example code ratio would suffer). The error boilerplate and main is typically hidden in order not to obscure the actual learning material which sadly leaves the Result<()> in sight which is counter productive to the learning process.

The main goals in regard to error handling in cookbook were:

• be correct and as close to real use cases
• be painless to implement (both for learners and cookbook contributors)
• require minimal boilerplate not to obscure the actual example / learning material

Once the error-chain seamed to provide the best trade-off for these particular requirements and I still hope that failure will reach maturity giving us minimal boilerplate.

At this point I'm not sure what would be the best way to proceed with the error handling, every approach that you have mentioned is perfectly valid with its own set of drawbacks and each strategy has its usage in given domains If possible I'd like to avoid advertising any particular error handling strategy and hid as much of it as possible, the reader will know that the hidden parts are not part of the example, just the implementation detail for the curious.
I'd love to get more feedback on this matter!

[AndyGauge]

Wow, there's a lot of thought in there that I can appreciate.

I personally have seen more and more movement towards a specified Enum of Error variants. I personally like that approach because it defines in the code the error types expected and doesn't require learning a library's macro to use. Here is Hyper's error enum. Here's a stackoverflow answer demonstrating how to implement an Enum.

The first book also gives this advice: defining your own error type. I think including something similar in the Error Handling section would help learners understand error-chain under the hood. If at the least we make it difficult for readers of the cookbook to miss doing this once to learn about managing their own errors, a novice would be prepared to use whichever library.

I also like the idea of taking error_chain out of every example that doesn't require it. Your example is one of them that slipped by review. error_chain is only supposed to be used when there are multiple error types returned.

What I want to see done is:

1. Identify the examples that are incorrectly using error_chain Remove Error-chain from examples not requiring it #506
2. Continue to consider failure and the possibility of including it in the future. It seems to be rapidly become de facto rust library for error management.
3. Look for a resource to manage multiple error types without magic, whether it be Box or Enum. Seeing how a library accomplishes this is important for knowing how to use a library.

[BurntSushi]

I can't say that I love the idea of writing explicit error handling in main [...snip...] and presenting it on nearly every example along with hand crafted error types. This would get rather verbose and would lead the examples to be quite noisy (error boilerplate vs actual example code ratio would suffer).

I think this might come down to folks weighting the negativity of boiler plate differently. To me, adding a 4 line main function to each example in exchange for making the example explicit and easy to understand, is a trade well worth making. Note that the main function probably never grows, or at least, doesn't grow much.

I also think that putting the main function in the "hidden" section of the code would be acceptable, or at least better than today. It's true that the Result is still implicit, but it would become explicit after looking at the hidden code. I think the existence of hidden code in the first place acknowledges that it's OK to make this trade in some cases. For example, examples are putting this in the hidden code:

error_chain! {
    foreign_links {
        Io(std::io::Error);
    }
}

My method allows you to remove that and the extern crate error_chain; at the top. So I think you actually wind up with fewer lines of code.

Separate from that is building out error enums. I do agree that this can become quite noisy. With that said, that pattern is in widespread use in the ecosystem in core libraries. However, it isn't always necessary. Box<Error> is a fine substitute in plenty of cases that should be suitable for very terse examples. io::Result<()> is probably also suitable in a large class of examples. For example, the csv crate's Error type has a From<csv::Error> for std::io::Error impl to make it seamless to use with io::Error. I expect other libraries might do the same.

[budziq]

I also like the idea of taking error_chain out of every example that doesn't require it. Your example is one of them that slipped by review. error_chain is only supposed to be used when there are multiple error types returned.

What I want to see done is:
...
Big 👍

I think this might come down to folks weighting the negativity of boiler plate differently.

It's admittedly my very subjective take on how we ought to present the teaching material. We established it without any prior teaching experience mostly based on "gut feeling" so nothing is set in stone and any ideas are welcome :)

My method allows you to remove that and the extern crate error_chain; at the top. So I think you actually wind up with fewer lines of code.

Yep we are totally on board with this idea as @AndyGauge mentioned its already on the agenda.

Box<Error> is a fine substitute in plenty of cases that should be suitable for very terse examples.

That is true but I'm worried a out two things:

1. What level of consistency between examples should we strive for?
2. Box<Error> is a very convenient pattern but I worry that mere fact of presenting it in the examples will act as blessing it as the "silver bullet" solution. Error handling is almost always dependent on additional factors and constraints which will not be present in the artificial environment of the cookbook. How do we prevent our readers from conflating boilerplate with idiomatic approach endorsement?

[BurntSushi]

Box<Error> is a very convenient pattern but I worry that mere fact of presenting it in the examples will act as blessing it as the "silver bullet" solution. Error handling is almost always dependent on additional factors and constraints which will not be present in the artificial environment of the cookbook. How do we prevent our readers from conflating boilerplate with idiomatic approach endorsement?

I don't think you can really solve this completely. Even if failure evolves a year from now with something that's based on the fixes to the standard library Error trait into something that gains near universal adoption, you will still always have cases where libraries want to create their own structured errors. At the end of the day, those error definitions aren't boiler plate; they are a key piece of your library's API. Some examples might call for defining those errors, or perhaps, you might consider specifically constructing examples in which defining your own custom error type is the right way forward. But other examples devolving to the use of trait objects (which ultimately is what failure is encouraging) seems fine to me, especially in examples that are command line programs.

Proper error handling in libraries can be difficult. While I think some examples devoted to it would be great, I do not think using trait objects in the majority of examples is a significant negative. If people wind up using Box<Error> in command line applications, then I don't think that's a big negative. If people wind up returning Box<Error> in their libraries (and possibly failure::Error in the future) in place of more structured errors, then that's more of a negative, but at that point, you're well into API design.

One additional note: Box<Error> is much less useful today than it will be once the Error trait is fixed with the aforementioned RFC. In particular, it will at that point be possible to downcast effectively and extract backtraces from it.

[AndyGauge]

Hi, I love this cookbook, and I feel fun with typing it on my vim screen :)Thank you.By the way, although the example shows a code which uses 'flate2' and 'tar' library, the code can't be compiled without "error_chain" crate. I found the problem by clicking the "expand" icon and could access the full source codes.It would be better if "error_chain" crate is specified explicitly(and also #[macro_use] is important one), and add some comment about it to avoid confusion to the beginner, or some description which informs reader to click the "expand" icon to get a whole working code.Thanks!

@gkm2164

[najamelan]

In the mean time, error-chain is unmaintained, and failure has been deprecated. Yet this cookbook still shows error-chain all while the introduction says:

Error handling in Rust is robust when done correctly, but in today's Rust it requires a fair bit of boilerplate. Because of this one often sees Rust examples filled with unwrap calls instead of proper error handling.

Since these recipes are intended to be reused as-is and encourage best practices, they set up error handling correctly when there are Result types involved.

The basic pattern we use is to have a fn main() -> Result.

I think this cookbook should have dedicated recipes about error handling right at the beginning. One for apps and one for libs. I would suggest that they show how to make a custom error type with thiserror for libs and how to use anyhow in application code. Explaining extras like why this is currently the recommended way. Maybe refer to crates like user-error and human-panic for apps as well.

I would follow the proposition of @BurntSushi of using an explicit main that handles the errors. There's nothing more hampering for learning than some invisible magic happening. Returning Result from main is an anti-pattern. It is functionally equivalent to unwrapping, that is to say worse than expect and only exists to hide the unwrapping from users in documentation so they wouldn't copy paste unwrap. It's no good in an actual application, so I don't think teaching materials should have it.

For aggregating errors, Box<dyn Error> is pretty good if you want to avoid boilerplate, but I would put a link just above every example that says, "for proper error handling techniques, see Chapter 1: Error Handling".

I know it's easy to have an opinion, but if there was consensus about this, I could put some time towards writing an error handling chapter.

[BurntSushi]

I would suggest that they show how to make a custom error type with thiserror for libs

I wouldn't recommend thiserror for libraries without qualification. It adds a fairly hefty dependency. It might be good to have an example using thiserror (and perhaps snafu), but the dominant method for defining errors in libraries should be without a derive helper (unless one becomes available in std).

There's nothing more hampering for learning than some invisible magic happening. Returning Result from main is an anti-pattern. It is functionally equivalent to unwrapping, that is to say worse than expect and only exists to hide the unwrapping from users in documentation so they wouldn't copy paste unwrap. It's no good in an actual application, so I don't think teaching materials should have it.

main -> Result works okay when you use anyhow::Result, but only because anyhow emits a human readable error message for its Debug format. (Precisely because main -> Result prints the debug representation.)

[najamelan]

I wouldn't recommend thiserror for libraries without qualification. It adds a fairly hefty dependency.

Are you referring to compilation time?