Extract an iterator that cleans redundant nested error text #383
Conversation
shepmaster
added
enhancement
✅ Deploy Preview for shepmaster-snafu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
shepmaster
force-pushed
the
cleaned-error-text
branch
from
a81961e to
07c3d45
Compare
| @@ -162,3 +162,59 @@ struct TestFunctionError; | |||
| fn procedural_macro_works_with_test_functions() -> Result<(), TestFunctionError> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| #[track_caller] | |||
| fn assert_cleaning_step(iter: &mut CleanedErrorText, text: &str, removed_text: &str) { | |||
There was a problem hiding this comment.
Do we have test coverage for the case where a message contains :, but the subsequent part of the string represents an unreachable cause not exposed via source? No clean-up should happen in these cases.
There was a problem hiding this comment.
Do you mean something like a leaf message saying “bad thing happened: restart your server”?
There was a problem hiding this comment.
Yes, or the case where the second part is indeed part of a sub-cause error message but the corresponding cause is deliberately not part of the error source chain.
There was a problem hiding this comment.
I'm not sure I follow that second case. Do you mean something like this pseudocode?
#[snafu(display("Outer is better than Inner"))] // doesn't use `{source}` but just happens to end with the exact same text
struct Outer { source: Inner }
#[snafu(display("Inner"))]
struct Inner;If so, I don't think there's any way to avoid that.
There was a problem hiding this comment.
I was thinking about the code below. I suspect the confusion is because one is unlikely to do this in snafu, but if for any reason we have a source error which we do not want to expose as an error, the error text cleaning routine should not clean the message.
#[derive(Debug, Snafu)]
#[snafu(display("Epic fail: {}", hidden_source))]
struct Outer {
hidden_source: Inner,
}
#[derive(Debug, Snafu)]
#[snafu(display("I AM ERROR"))]
struct Inner;The display message of an Outer should be "Epic fail: I AM ERROR", per the recommendations of the error handling WG.
The situation you just mentioned is a curious one, though! Could we have problems with the outer error's message ending with the exact message as its source-error message?
There was a problem hiding this comment.
we do not want to expose as an error
In that case, it would not be cleaned — by construction. This walks the Error::source chain to find the duplicate text. It'e effectively trying to retroactively apply the error handling WG's suggestion of (Error::source XOR "include it in Display) for errors that do both.
Calling Error::source on Outer will return None, so we will never get the Display string from Inner, so we will not use it to clean Outer's text.
Could we have problems with the outer error's message ending with the exact message as its source-error message?
It's always possible. That's why snafu::Report offers a way to disable the cleaning. I do think that it is pretty unlikely, however.
This allows the cleaning logic to be reused in other contexts, such as an HTML or JSON error report.
shepmaster
force-pushed
the
cleaned-error-text
branch
from
07c3d45 to
e4e57ea
Compare
This allows the cleaning logic to be reused in other contexts, such as an HTML or JSON error report.