Prevent duplicated error information in "wrapping errors" example#1375
Merged
marioidival merged 1 commit intorust-lang:masterfrom Sep 28, 2020
d-e-s-o:master
Merged
Prevent duplicated error information in "wrapping errors" example#1375marioidival merged 1 commit intorust-lang:masterfrom d-e-s-o:master
marioidival merged 1 commit intorust-lang:masterfrom
d-e-s-o:master
Conversation
The "Wrapping errors" example, which illustrates how to wrap errors, is using the wrapped error in the text that is displayed as well as the implementation of source(). In so doing it effectively duplicates the error information. Such behavior does not play nice with error reporting crates such as anyhow, eyre, and others that make use of both this data. It is also questionable at best from a logical point of view, because, as the name suggests, the source should be a lower level error that is the cause of this one. With this change we update the documentation, suggesting the usage of a custom error string describing at a higher level what went wrong. To keep the example simple, we did not include the actual string in the error (a comment already suggests that this could be done), although that probably should be done in real life to provide additional context. An alternative approach would be to not return a source, but I figured that it's sort of desired for the illustration of wrapping an error.
|
r? @marioidival (rust_highfive has picked a reviewer for you, use r? to override) |
Contributor
Author
|
See for instance this pull request that fixes exactly such a duplication of error information. |
Contributor
Author
|
Any chance this change could get a review? |
Contributor
Author
|
r? |
marioidival
approved these changes
Sep 28, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The "Wrapping errors" example, which illustrates how to wrap errors, is
using the wrapped error in the text that is displayed as well as the
implementation of source(). In so doing it effectively duplicates the
error information. Such behavior does not play nice with error reporting
crates such as anyhow, eyre, and others that make use of both this data.
It is also questionable at best from a logical point of view, because,
as the name suggests, the source should be a lower level error that is
the cause of this one.
With this change we update the documentation, suggesting the usage of a
custom error string describing at a higher level what went wrong. To
keep the example simple, we did not include the actual string in the
error (a comment already suggests that this could be done), although
that probably should be done in real life to provide additional context.
An alternative approach would be to not return a source, but I figured
that it's sort of desired for the illustration of wrapping an error.