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

Don't print inner error when returning as source #2533

Merged
merged 2 commits into from
Mar 1, 2022

Conversation

thomaseizinger
Copy link
Contributor

According to rust-lang/project-error-handling#44 (comment), we should not be printing the inner error AND returning it as source. Libraries like anyhow will traverse the chain of source errors and build up a composed error message. Printing it and returning the same error from source results in duplicate error messages in that case.

According to rust-lang/project-error-handling#44 (comment), we should not be printing the inner error AND returning it as source. Libraries like `anyhow` will traverse the chain of `source` errors and build up a composed error message. Printing it and returning the same error from `source` results in duplicate error messages in that case.
@mxinden
Copy link
Member

mxinden commented Feb 28, 2022

Thanks for the pointer. I wasn't aware of this convention.

How about taking this one step further and deriving Error and Display for TransportError via thiserror?

@thomaseizinger
Copy link
Contributor Author

Thanks for the pointer. I wasn't aware of this convention.

How about taking this one step further and deriving Error and Display for TransportError via thiserror?

I support the idea (there is a ticket for it I think?). Can we still merge this fix and we do the thiserror migration later? :)

@mxinden
Copy link
Member

mxinden commented Mar 1, 2022

(there is a ticket for it I think?)

#2509

@mxinden mxinden merged commit d1f472b into master Mar 1, 2022
@thomaseizinger thomaseizinger deleted the fix/no-duplicate-print branch May 16, 2022 11:20
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

Successfully merging this pull request may close these issues.

2 participants