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

fix(tonic): don't include error's cause in Display impl #633

Merged
merged 1 commit into from
May 27, 2021

Conversation

davidpdrsn
Copy link
Member

@davidpdrsn davidpdrsn commented May 7, 2021

At Embark we have a little helper function that converts a &dyn std::error::Error into a String by walking the full chain of sources
(with std::error::Error::source) and joining them into a String.

We use that where we log errors to get as much information as possible
about whats causing an error. Works particularly well with anyhow's
.context() method.

However since tonic::transport::Error include its cause in their
Display impl we get the sources more than once.

As the cause can already be obtained through std::error::Error::source
no information should be lost by doing this.

Fixes #632

At Embark we have a little helper function that converts a `&dyn
std::error::Error` into a `String` by walking the full chain of sources
(with `std::error::Error::source`) and joining them into a `String`.

We use that where we log errors to get as much information as possible
about whats causing an error. Works particularly well with anyhow's
`.context()` method.

However since `tonic::transport::Error` include its cause in their
`Display` impl we get the sources more than once.

As the cause can already be obtained through `std::error::Error::source`
no information should be lost by doing this.

Fixes #632
@davidpdrsn davidpdrsn merged commit 31a3468 into master May 27, 2021
@davidpdrsn davidpdrsn deleted the david/transport-error-display-fix branch May 27, 2021 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

impl Display for transport::Error includes the source
2 participants