rust-lang / project-error-handling

Error handling project group
Apache License 2.0
268 stars 18 forks source link

When should `Display` of `Error` also print the cause? #23

Open mahkoh opened 3 years ago

mahkoh commented 3 years ago

anyhow prints the cause chain only with {:#} but tokio-postgres always prints the cause. This causes rust-postgres errors wrapped in anyhow errors to be printed twice if at all.

            log::error!("postgres connection failed: {:#}", e);
postgres connection failed: db error: FATAL: terminating connection due to administrator command: FATAL: terminating connection due to administrator command

https://github.com/sfackler/rust-postgres/blob/8f2ae8647bd2de68a77d85bfb005de3cfdd548a3/tokio-postgres/src/error/mod.rs#L396-L398

https://github.com/dtolnay/anyhow/blob/5e4177fc4c9e211feb90f1d81ea0ef183f171fa7/src/fmt.rs#L9-L13

Enet4 commented 3 years ago

The subject has been brought up a few times before. Relevant SO question. The short answer is that both design decisions (as in, whether to print the source error on the Display impl) have upsides and downsides; some error handling libraries impose an opinion on this (e.g. anyhow/eyre) whereas others currently do not (SNAFU); and although things appear to be turning towards letting the job of printing the source error to the reporter, this is still not a well established consensus, to the point that this has not been accepted to become part of the API guidelines (rust-lang/api-guidelines#210), mostly because it would still break some expectations.

Nevertheless, it is good for the subject to be brought up here, which might serve as a catalyst towards this consensus.

yaahc commented 3 years ago

ty @Enet4, that's a very good summary.

To add some more to that, we're currently leaning towards the reporter based design, but it's too early to start pushing for it across the ecosystem because there are quite a few edge cases still where context for errors can be discarded because no reporter is ever used. Thing's like logging or printing errors directly, or panicking with Result's containing E: Error types that haven't been converted to a Reporter will fail to introduce the party that is responsible for iterating over the sources and printing all of the errors. We're working on some ideas for how we can fix this and get the globally defined reporters utilized consistently when printing errors, but they're still a long way off.

For now, the recommendation is to always do one or the other. If you're going to print your sources, you should not return them via the source function and vice versa.