shepmaster / snafu

Easily assign underlying errors into domain-specific errors while adding context
https://docs.rs/snafu/
Apache License 2.0
1.4k stars 60 forks source link

Automatically detect and remove nested error messages in Report output #363

Closed shepmaster closed 1 year ago

shepmaster commented 1 year ago

An example reqwest error:

error sending request for url (https://rust-lasagna.org/): error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known

Caused by these errors (recent errors listed first):
  1: error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known
  2: dns error: failed to lookup address information: nodename nor servname provided, or not known
  3: failed to lookup address information: nodename nor servname provided, or not known

We could attempt to remove the substring of the next error:

error sending request for url (https://rust-lasagna.org/): error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known
                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Proposed algorithm would be to use str::trim_end_matches, trim trailing whitespace, then a trailing :.

Then the output would look something like...

error sending request for url (https://rust-lasagna.org/)

Caused by these errors (recent errors listed first):
  1: error trying to connect
  2: dns error
  3: failed to lookup address information: nodename nor servname provided, or not known
Enet4 commented 1 year ago

Well, so long as the algorithm can be disabled in controlled scenarios where we know this doesn't happen, it sounds like a nice idea. I just wonder what the phasing out strategy would be, considering that this would also hide the fact that some libraries are not following the official recommendation from the error handling WG.

shepmaster commented 1 year ago

so long as the algorithm can be disabled in controlled scenarios where we know this doesn't happen

I'm not sure I follow your meaning — can you expand on it a bit?

I'm pretty sure that to do this we would need allocation, so we will rely on the standard library. In that case we could use an environment variable to turn it off.

this would also hide the fact that some libraries are not following the official recommendation

One possibility would be to track if we made any changes and if so append another line (maybe even a symbol next to the offending lines?) to explain what happened and how to remove it:

Caused by these errors (recent errors listed first):
  1: 
  2:
  3:

  Note: some redundant information has been removed from the output. Set SNAFU_COOL_ENV_VAR=1 to disable this behavior.
Enet4 commented 1 year ago

I'm not sure I follow your meaning — can you expand on it a bit?

I'm thinking that eventually most dependencies' errors would be well behaved in terms of their Display impl, so in the event that they all are, the check for trailing messages would be introducing an unnecessary overhead. I may be overthinking this though, considering that this only happens at error reporting and does not have to be super fast... :thinking:

The redundant information removal notice seems fine too.

shepmaster commented 1 year ago
fn main() {
    let orig = vec![
        "error sending request for url (https://rust-lasagna.org/): error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known",
        "error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known",
        "dns error: failed to lookup address information: nodename nor servname provided, or not known",
        "failed to lookup address information: nodename nor servname provided, or not known",
    ];

    let mut clean = Vec::with_capacity(orig.len());
    clean.extend(
        orig.iter()
            .zip(orig.iter().skip(1))
            .map(|(a, b)| a.trim_end_matches(b).trim_end().trim_end_matches(":")),
    );
    clean.extend(orig.last());

    dbg!(clean);
}