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

impl Display for transport::Error includes the source #632

Closed
davidpdrsn opened this issue May 7, 2021 · 0 comments · Fixed by #633
Closed

impl Display for transport::Error includes the source #632

davidpdrsn opened this issue May 7, 2021 · 0 comments · Fixed by #633
Labels

Comments

@davidpdrsn
Copy link
Member

davidpdrsn commented May 7, 2021

This leads to the Display representation being quite large in some cases. I think a better approach is to include the source in Error::source so its up to users how much information they want to show.

davidpdrsn added a commit that referenced this issue 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
davidpdrsn added a commit that referenced this issue May 27, 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
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 a pull request may close this issue.

1 participant