From e4e57eacec684c99738230a2b95b6b98c39a0816 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Wed, 5 Jul 2023 07:49:13 -0400 Subject: [PATCH] Extract an iterator that cleans redundant nested error text This allows the cleaning logic to be reused in other contexts, such as an HTML or JSON error report. --- src/lib.rs | 2 + src/report.rs | 122 +++++++++++++++++++++++++++++++++++++++--------- tests/report.rs | 58 ++++++++++++++++++++++- 3 files changed, 159 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3e3da5f1..102d139b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -272,6 +272,8 @@ mod error_chain; pub use crate::error_chain::*; mod report; +#[cfg(feature = "std")] +pub use report::CleanedErrorText; pub use report::{Report, __InternalExtractErrorType}; doc_comment::doc_comment! { diff --git a/src/report.rs b/src/report.rs index ac277e87..b9b152f3 100644 --- a/src/report.rs +++ b/src/report.rs @@ -272,33 +272,23 @@ impl<'a> ReportFormatter<'a> { fn cleaned_error_trace(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { const NOTE: char = '*'; - let mut original_messages = ChainCompat::new(self.0).map(ToString::to_string); - let mut prev = original_messages.next(); - - let mut cleaned_messages = vec![]; let mut any_cleaned = false; let mut any_removed = false; - for msg in original_messages { - if let Some(mut prev) = prev { - let cleaned = prev.trim_end_matches(&msg).trim_end().trim_end_matches(':'); - if cleaned.is_empty() { + let cleaned_messages: Vec<_> = CleanedErrorText::new(self.0) + .flat_map(|(_, mut msg, cleaned)| { + if msg.is_empty() { any_removed = true; - // Do not add this to the output list - } else if cleaned != prev { - any_cleaned = true; - let cleaned_len = cleaned.len(); - prev.truncate(cleaned_len); - prev.push(' '); - prev.push(NOTE); - cleaned_messages.push(prev); + None } else { - cleaned_messages.push(prev); + if cleaned { + any_cleaned = true; + msg.push(' '); + msg.push(NOTE); + } + Some(msg) } - } - - prev = Some(msg); - } - cleaned_messages.extend(prev); + }) + .collect(); let mut visible_messages = cleaned_messages.iter(); @@ -357,6 +347,94 @@ fn trace_cleaning_enabled() -> bool { !DISABLED.get(|| env::var_os(SNAFU_RAW_ERROR_MESSAGES).map_or(false, |v| v == "1")) } +/// An iterator over an Error and its sources that removes duplicated +/// text from the error display strings. +/// +/// It's common for errors with a `source` to have a `Display` +/// implementation that includes their source text as well: +/// +/// ```text +/// Outer error text: Middle error text: Inner error text +/// ``` +/// +/// This works for smaller errors without much detail, but can be +/// annoying when trying to format the error in a more structured way, +/// such as line-by-line: +/// +/// ```text +/// 1. Outer error text: Middle error text: Inner error text +/// 2. Middle error text: Inner error text +/// 3. Inner error text +/// ``` +/// +/// This iterator compares each pair of errors in the source chain, +/// removing the source error's text from the containing error's text: +/// +/// ```text +/// 1. Outer error text +/// 2. Middle error text +/// 3. Inner error text +/// ``` +#[cfg(feature = "std")] +pub struct CleanedErrorText<'a>(Option>); + +#[cfg(feature = "std")] +impl<'a> CleanedErrorText<'a> { + /// Constructs the iterator. + pub fn new(error: &'a dyn crate::Error) -> Self { + Self(Some(CleanedErrorTextStep::new(error))) + } +} + +#[cfg(feature = "std")] +impl<'a> Iterator for CleanedErrorText<'a> { + /// The original error, the display string and if it has been cleaned + type Item = (&'a dyn crate::Error, String, bool); + + fn next(&mut self) -> Option { + use std::mem; + + let mut step = self.0.take()?; + let mut error_text = mem::replace(&mut step.error_text, Default::default()); + + match step.error.source() { + Some(next_error) => { + let next_error_text = next_error.to_string(); + + let cleaned_text = error_text + .trim_end_matches(&next_error_text) + .trim_end() + .trim_end_matches(':'); + let cleaned = cleaned_text.len() != error_text.len(); + let cleaned_len = cleaned_text.len(); + error_text.truncate(cleaned_len); + + self.0 = Some(CleanedErrorTextStep { + error: next_error, + error_text: next_error_text, + }); + + Some((step.error, error_text, cleaned)) + } + None => Some((step.error, error_text, false)), + } + } +} + +#[cfg(feature = "std")] +struct CleanedErrorTextStep<'a> { + error: &'a dyn crate::Error, + error_text: String, +} + +#[cfg(feature = "std")] +impl<'a> CleanedErrorTextStep<'a> { + fn new(error: &'a dyn crate::Error) -> Self { + let error_text = error.to_string(); + Self { error, error_text } + } +} + #[doc(hidden)] pub trait __InternalExtractErrorType { type Err; diff --git a/tests/report.rs b/tests/report.rs index 7a1cd7bb..15bf02ab 100644 --- a/tests/report.rs +++ b/tests/report.rs @@ -1,4 +1,4 @@ -use snafu::{prelude::*, IntoError, Report}; +use snafu::{prelude::*, CleanedErrorText, IntoError, Report}; macro_rules! assert_contains { (needle: $needle:expr, haystack: $haystack:expr) => { @@ -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) { + let (error, actual_text, actual_cleaned) = + iter.next().expect("Iterator unexpectedly exhausted"); + let actual_original_text = error.to_string(); + + let original_text = [text, removed_text].concat(); + let cleaned = !removed_text.is_empty(); + + assert_eq!(original_text, actual_original_text); + assert_eq!(text, actual_text); + assert_eq!(cleaned, actual_cleaned); +} + +#[test] +fn cleaning_a_leaf_error_changes_nothing() { + #[derive(Debug, Snafu)] + #[snafu(display("But I am only C"))] + struct C; + + let c = C; + let mut iter = CleanedErrorText::new(&c); + + assert_cleaning_step(&mut iter, "But I am only C", ""); + assert!(iter.next().is_none()); +} + +#[test] +fn cleaning_nested_errors_removes_duplication() { + #[derive(Debug, Snafu)] + #[snafu(display("This is A: {source}"))] + struct A { + source: B, + } + + #[derive(Debug, Snafu)] + #[snafu(display("And this is B: {source}"))] + struct B { + source: C, + } + + #[derive(Debug, Snafu)] + #[snafu(display("But I am only C"))] + struct C; + + let a = A { + source: B { source: C }, + }; + let mut iter = CleanedErrorText::new(&a); + + assert_cleaning_step(&mut iter, "This is A", ": And this is B: But I am only C"); + assert_cleaning_step(&mut iter, "And this is B", ": But I am only C"); + assert_cleaning_step(&mut iter, "But I am only C", ""); + assert!(iter.next().is_none()); +}