Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error messages don't compose well with anyhow #533

Closed
davepacheco opened this issue Oct 16, 2021 · 3 comments
Closed

error messages don't compose well with anyhow #533

davepacheco opened this issue Oct 16, 2021 · 3 comments

Comments

@davepacheco
Copy link

Describe the bug

I've been using @dtolnay's anyhow crate as a great way to annotate errors with context about was happening when an error was generated. I found that with uuid, though, it generates duplicate messages.

To Reproduce

Steps to reproduce the behavior:

  1. Clone https://github.com/davepacheco/test-uuid-anyhow (or see the code below)
  2. cargo run

Here's the relevant code:

    let f = "foo".parse::<Uuid>().context("parsing").unwrap_err();
    ...
    eprintln!("    alt format: {:#}", f);

Expected behavior

I expected this to print:

parsing: invalid length: expected one of [36, 32], found 3

but it actually prints:

parsing: invalid length: expected one of [36, 32], found 3: invalid length: expected one of [36, 32], found 3

Screenshots

The rest of the program shows exactly what the error chain looks like. Here's the output:

top-level (anyhow) error:
     formatted: parsing
    alt format: parsing: invalid length: expected one of [36, 32], found 3: invalid length: expected one of [36, 32], found 3
anyhow debug format:
parsing

Caused by:
    0: invalid length: expected one of [36, 32], found 3
    1: invalid length: expected one of [36, 32], found 3

CAUSE CHAIN
chain 0: Error { context: "parsing", source: Error(Parser(InvalidLength { expected: Any([36, 32]), found: 3 })) }
     formatted: parsing
    alt format: parsing


chain 1: Error(Parser(InvalidLength { expected: Any([36, 32]), found: 3 }))
     formatted: invalid length: expected one of [36, 32], found 3
    alt format: invalid length: expected one of [36, 32], found 3


chain 2: InvalidLength { expected: Any([36, 32]), found: 3 }
     formatted: invalid length: expected one of [36, 32], found 3
    alt format: invalid length: expected one of [36, 32], found 3


Specifications (please complete the following information):

  • Target
  • Version [e.g. 1.0]
  • Features Enabled

This is with uuid 0.8.2, anyhow 1.0.44, rust 1.52.0, on MacOS 11.6 (Big Sur). I haven't enabled any features. (You can see the complete details in the Cargo.toml and Cargo.lock in the repo I linked to.)

Additional context

anyhow uses the alternate Display format to print the whole cause chain as a single message. I like this a lot because it prints errors the way most standard Unix tools do.

I believe the way it works is it follows the cause chain using std::error::Error::source() and prints out the message associated with each one. This seems pretty reasonable.

I think the problem is that the uuid crate uses a few levels of Error (i.e., a cause chain), and Display on the higher level impl delegates to the next level. So if one walks the cause chain and prints each one out, you get the duplicated message. This structure doesn't really seem right, though I'm not sure the best fix because I'm not sure why the indirection.

@KodrAus
Copy link
Member

KodrAus commented Oct 27, 2021

Thanks for the report @davepacheco! It looks like we need to take a fresh look at our error handling story. If this is something you’re interested in exploring please feel free to dig into it!

@KodrAus
Copy link
Member

KodrAus commented Oct 30, 2021

Alrighty, I’ve reworked the internal shape of uuid::Error so it now reports its inner context through Display instead of source. That should make it work better with anyhow.

@KodrAus KodrAus closed this as completed Oct 30, 2021
@davepacheco
Copy link
Author

Thanks for doing that! It looks great. If I switch the test repo to use commit 2925da3 (current tip of main), the output looks like this:

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/uuid-error`
top-level (anyhow) error:
     formatted: parsing
    alt format: parsing: invalid length: expected one of [36, 32], found 3
anyhow debug format:
parsing

Caused by:
    invalid length: expected one of [36, 32], found 3

CAUSE CHAIN
chain 0: Error { context: "parsing", source: Error(InvalidLength { expected: Any([36, 32]), found: 3 }) }
     formatted: parsing
    alt format: parsing


chain 1: Error(InvalidLength { expected: Any([36, 32]), found: 3 })
     formatted: invalid length: expected one of [36, 32], found 3
    alt format: invalid length: expected one of [36, 32], found 3

which is perfect. Thanks!

Any idea when this will wind up in a release on crates.io?

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

No branches or pull requests

2 participants