Do people not care about printable error chains? a.k.a. How to nicely implement Display for an error?

[faern]

This post became much longer than I expected. TLDR bullet list:

• Rust needs much better official API guideline for how to design good error types.
• How should one implement Display for errors that are essentially just a sum type of other errors.

Ever since error-chain was the hot new kid on the block I have logged/printed/presented my errors as what has been the equivalent of what error-chain called the DisplayChain at the time. Meaning when I print an error:

log::error!("{}", error.display_chain());

I get something like the following in my log:

[2019-12-04 18:32][some_module][ERROR] Error: Unable to start daemon
Caused by: Failed to initialize firewall module
Caused by: Unable to open /dev/pf
Caused by: Permission denied

When error-chain became unmaintained I started doing it manually for a while. Then came err-context which did it for me again, this time with configurable separator ("\nCaused by: ").

For me it's the user facing version of a backtrace. It makes it sooo easy to see what fails in a readable manner. It makes it easy for developers to track down what went wrong and it's even somewhat understandable for many end users.

However, the above output of course depends on error types correctly implementing something good as their Display implementation. The decision on what to print in the Display implementation is what this post is about really. For leaf errors without any sources, like std::num::ParseIntError it's easy, just say what went wrong. The hard part comes when writing errors for methods that can fail due to multiple fallible operations inside them, even with different error types for each. For this it's common to have an error sum type like:

enum MyLibError {
    Io(std::io::Error),
    Parse(std::num::ParseIntError),
}

What should the Display implementation for this error be? Silly as it may sound, this is probably the biggest headache I have with Rust at the moment.

I have seen many instances where the above type of error includes the Display impl of the source error. Either directly without added context , or with some extra context around. The reason I chose to reference csv is that it has been around for a long time, has a huge amount of downloads and is developed my someone often highly regarded as writing good Rust code. The problem of doing this is that the printed error chain now has duplicated and redundant information. Just a few days ago I got the following printed from a program I have that uses tonic for gRPC:

Error: Failed to connect to admin gRPC endpoint
Caused by: Client: error trying to connect: Connection refused (os error 111)
Caused by: error trying to connect: Connection refused (os error 111)
Caused by: Connection refused (os error 111)

And yeah... As you can see it's not really very nice at all. It must be that the developers who write these errors never print them in the way I do, no? How do they print their errors?

One thing I'm personally doing for sum error types is to be more detailed, like:

// Or more detailed sometimes:
enum MyLibError {
    ReadSettings(std::io::Error),
    WriteSettings(std::io::Error),
    ParseSettings(std::num::ParseIntError),
}

Then the Display impl can be something like:

match self {
    ReadSettings => "Failed to read settings".fmt(f),
    WriteSettings => "Failed to write settings".fmt(f),
    ParseSettings => "Failed to parse settings".fmt(f),
}

And I never use the source of my error in my own error type, except for returning it from the source() method of course.

I have been thinking about this for a very long time, but never got to write anything until now, when I read this blog post: Thoughts on Error Handling in Rust · Lukasʼ Blog. Which is a great post! My only concern is, what would such an automatic error sum type as they discuss Display?

We can make the sum types just proxy the Display text from their sources iff we can we modify the standard library Error trait to have something like Error::is_just_a_sum_of_other_errors(&self) -> bool. And then we can get Error::display_chain(&self, separator: &str) -> DisplayChain into the standard library as well. Where DisplayChain would work like in error-chain/err-context except it would skip errors where is_just_a_sum_of_other_errors is true. This is a very half baked solution with silly example names, but I just mean to communicate the idea here.

Since the set of errors with valuable display information seems to be a strict subset of all errors, there must be some way to know whether or not to print a given instance or skip to the source.

[dtolnay]

And I never use the source of my error in my own error type, except for returning it from the source() method of course.

This is the correct answer.

Using thiserror's attribute syntax for convenience, the contents of the error type should appear either in the message:

#[derive(Error)]
enum Error {
    #[error("Failed to read settings: {0}")]
    ReadSettings(io::Error),
}

or as a source:

#[derive(Error)]
pub enum Error {
    #[error("Failed to read settings")]
    ReadSettings(#[source] io::Error),
}

but never both. Whatever error in the tonic crate was printing as "error trying to connect: ..." was doing this wrong.

For error layers that have no useful information to add beyond the inner error value that they contain, the approach that I have found best is for the outer error's Display impl and source method to both forward to the inner error's. Thiserror implements this as:

#[derive(Error)]
pub enum Error {
    #[error("Failed to read settings")]
    ReadSettings(#[source] io::Error),

    // Anything else might have failed, we have nothing
    // useful to add at this level.
    #[error(transparent)]
    Other(anyhow::Error),
}

This is better than "{0}" (which would cause the message to be repeated in the cause chain, as above) or something pointless like "Something else failed" or "Internal error" which has no benefit being in the cause chain.

[faern]

I like the transparent part! There you do use the source as the parent's Display impl, which went against the other rule we spoke about. But you also skip one error in the source() chain instead, so the printable chain becomes correct. I like that solution.

It matches very well with the reasoning that a MyError::Io(io_error) instance is an I/O error instead of it containing an I/O error.