Should an Error with a source include that source in the Display output?

user_310597 0
Viewed 852

I have an error type that impls the Error trait, and it wraps an underlying error cause, so the source method returns Some(source). I want to know whether the Display impl on my error type should include a description of that source error, or not.

I can see two options:

  1. Yes, include source in Display output, e.g. "Error opening database: No such file"

This makes it easy to print a whole error chain just by formatting with "{}" but impossible to only display the error itself without the underlying chain of source errors. Also it makes the source method a bit pointless and gives client code no choice on how to format separation between each error in the chain. Nevertheless this choice seems common enough in example code I have found.

  1. No, just print the error itself e.g. "Error opening database" and leave it to client code to traverse and display source if it wants to include that in the output.

This gives client code the choice of whether to display just the surface error or the whole chain, and in the latter case how to format separation between each error in the chain. It leaves client code with the burden of iterating through the chain, and I haven't yet fallen upon a canonical utility for conveniently formatting an error chain from errors that each only Display themselves excluding source. (So of course I have my own.)

The snafu crate (which I really like) seems to hint at favoring option 2, in that an error variant with a source field but no display attribute defaults to formatting Display output that does not include source.

Maybe my real question here is: What is the purpose of the source method? Is it to make formatting error chains more flexible? Or should Display really output everything that should be user-visible about an error, and source is just there for developer-visible purposes?

I would love to see some definitive guidance on this, ideally in the documentation of the Error trait.

#[derive(Debug)]
enum DatabaseError {
    Opening { source: io::Error },
}

impl Error for DatabaseError {
    fn source(&self) -> Option<&(dyn Error + 'static)> {
        match self {
            DataBaseError::Opening { source } => Some(source),
        }
    }
}

impl fmt::Display for DatabaseError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            DatabaseError::Opening { source } => {
                // ??? Should we include the source?
                write!(f, "Error opening database: {}", source)

                // ??? Or should we leave it to the caller to call .source()
                //     if they want to include that in the error description?
                write!(f, "Error opening database")
            }
        }
    }
}
rust error-handling
1 Answers

The two options of whether to print the source error on a Display implementation creates two schools of design. This answer will explain the two while objectively stating their key differences and clarifying a few possible misconceptions in the way.

Design 1: Yes, include source on your Display impl

Example with SNAFU:

#[derive(Debug, Snafu)]
enum Error {
    #[snafu(display("Could not read data set token: {}", source))]
    ReadToken {
        #[snafu(backtrace)]
        source: ReadDataSetError,
    },
}

The key advantage, as already mentioned in the question, is that providing the full amount of information is as simple as just printing the error value.

eprintln!("[ERROR] {}", err);

It is simple and easy, requiring no helper functions for reporting the error, albeit with the lack of presentation flexibility. Without string manipulation, a chain of colon-separated errors is what you will always get.

[ERROR] Could not read data set token: Could not read item value: Undefined value length of element tagged (5533,5533) at position 3548

Design 2: No, leave out source on your Display impl

#[derive(Debug, Snafu)]
enum Error {
    #[snafu(display("Could not read data set token"))]
    ReadToken {
        #[snafu(backtrace)]
        source: ReadDataSetError,
    },
}

While this will not give you the full information with a single line print like before, you can leave that task to a project-wide error reporter. This also grants the consumer of the API greater flexibility on error presentation.

A simple example follows. Additional logic would be required for presenting the error's backtrace.

fn report<E: 'static>(err: E)
where
    E: std::error::Error,
    E: Send + Sync,
{
    eprintln!("[ERROR] {}", err);
    if let Some(cause) = err.source() {
        eprintln!();
        eprintln!("Caused by:");
        for (i, e) in std::iter::successors(Some(cause), |e| e.source()).enumerate() {
            eprintln!("   {}: {}", i, e);
        }
    }
}

Playground

It's also worth considering the interest of integrating with opinionated libraries. That is, certain crates in the ecosystem may have already made an assumption about which option to choose. In anyhow, error reports will already traverse the error's source chain by default. When using anyhow for error reporting, you should not be adding source, otherwise you may come up with an irritating list of repeated messages:

[ERROR] Could not read data set token: Could not read item value: Undefined value length of element tagged (5533,5533) at position 3548

Caused by:
   0: Could not read item value: Undefined value length of element tagged (5533,5533) at position 3548
   1: Undefined value length of element tagged (5533,5533) at position 3548

Likewise, the eyre library provides a customizable error reporting abstraction, but existing error reporters in the eyre crate ecosystem also assume that the source is not printed by the error's Display implementation.

So, which one?

Thanks to the efforts of the Error Handling project group, a key guideline regarding the implementation of Display was proposed in early 2021:

An error type with a source error should either return that error via source or include that source's error message in its own Display output, but never both.

That would be the second design: avoid appending the source's error message in your Display implementation. For SNAFU users, this currently means that applications will need to bring in an error reporter until one is made available directly in the snafu crate. As the ecosystem is yet to mature around this guideline, one may still find error utility crates lacking support for error reporting in this manner.

In either case...

This decision only plays a role in error reporting, not in error matching or error handling in some other manner. The existence of the source method establishes a chain-like structure on all error types, which can be exploited in pattern matching and subsequent flow control of the program. The Error::source method has a purpose in the ecosystem, regardless of how errors are reported.

In addition, it's ultimately up to the developers to choose how to design their errors and respective Display implementations, although once it starts integrating with other components, following the guideline will be the right way towards consistent error reporting.

What about Rust API guidelines?

The Rust API guidelines do not present an opinion about Display in errors, other than C-GOOD-ERR, which only states that the error type's Display message should be "lowercase without trailing punctuation, and typically concise". There is a pending proposal to update this guideline, instructing developers to exclude source in their Display impl. However, the pull request was created before the guideline was proposed, and has not been updated since then (at the time of writing).

See also:

user_12332510
Related