Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

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

Open
mahkoh opened this issue Dec 7, 2020 · 2 comments
Open

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

mahkoh opened this issue Dec 7, 2020 · 2 comments

Comments

@mahkoh
Copy link

mahkoh commented Dec 7, 2020

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
Copy link

Enet4 commented Dec 7, 2020

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
Copy link
Member

yaahc commented Dec 7, 2020

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants