Skip to content

Commit

Permalink
Extract an iterator that cleans redundant nested error text
Browse files Browse the repository at this point in the history
This allows the cleaning logic to be reused in other contexts, such as an HTML
or JSON error report.
  • Loading branch information
shepmaster committed Jul 6, 2023
1 parent d993c94 commit e4e57ea
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 23 deletions.
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down
122 changes: 100 additions & 22 deletions src/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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<CleanedErrorTextStep<'a>>);

#[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<Self::Item> {
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;
Expand Down
58 changes: 57 additions & 1 deletion tests/report.rs
Original file line number Diff line number Diff line change
@@ -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) => {
Expand Down Expand Up @@ -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());
}

0 comments on commit e4e57ea

Please sign in to comment.