Skip to content

Commit

Permalink
Automatically detect and remove nested error messages in Report output
Browse files Browse the repository at this point in the history
Closes #363
  • Loading branch information
shepmaster committed Nov 18, 2022
1 parent e40bcca commit ed78681
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 5 deletions.
118 changes: 113 additions & 5 deletions src/report.rs
Expand Up @@ -219,6 +219,35 @@ struct ReportFormatter<'a>(&'a dyn crate::Error);

impl<'a> fmt::Display for ReportFormatter<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
#[cfg(feature = "std")]
{
if trace_cleaning_enabled() {
self.cleaned_error_trace(f)?;
} else {
self.error_trace(f)?;
}
}

#[cfg(not(feature = "std"))]
{
self.error_trace(f)?;
}

#[cfg(feature = "unstable-provider-api")]
{
use core::any;

if let Some(bt) = any::request_ref::<crate::Backtrace>(self.0) {
writeln!(f, "\nBacktrace:\n{}", bt)?;
}
}

Ok(())
}
}

impl<'a> ReportFormatter<'a> {
fn error_trace(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
writeln!(f, "{}", self.0)?;

let sources = ChainCompat::new(self.0).skip(1);
Expand All @@ -236,19 +265,98 @@ impl<'a> fmt::Display for ReportFormatter<'a> {
writeln!(f, "{:3}: {}", i, source)?;
}

#[cfg(feature = "unstable-provider-api")]
{
use core::any;
Ok(())
}

if let Some(bt) = any::request_ref::<crate::Backtrace>(self.0) {
writeln!(f, "\nBacktrace:\n{}", bt)?;
#[cfg(feature = "std")]
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() {
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);
} else {
cleaned_messages.push(prev);
}
}

prev = Some(msg);
}
cleaned_messages.extend(prev);

let mut visible_messages = cleaned_messages.iter();

let head = match visible_messages.next() {
Some(v) => v,
None => return Ok(()),
};

writeln!(f, "{}", head)?;

match cleaned_messages.len() {
0 | 1 => {}
2 => writeln!(f, "\nCaused by this error:")?,
_ => writeln!(f, "\nCaused by these errors (recent errors listed first):")?,
}

for (i, msg) in visible_messages.enumerate() {
// Let's use 1-based indexing for presentation
let i = i + 1;
writeln!(f, "{:3}: {}", i, msg)?;
}

if any_cleaned || any_removed {
write!(f, "\nNOTE: ")?;

if any_cleaned {
write!(
f,
"Some redundant information has been removed from the lines marked with {}. ",
NOTE,
)?;
} else {
write!(f, "Some redundant information has been removed. ")?;
}

writeln!(
f,
"Set {}=1 to disable this behavior.",
SNAFU_RAW_ERROR_MESSAGES,
)?;
}

Ok(())
}
}

#[cfg(feature = "std")]
const SNAFU_RAW_ERROR_MESSAGES: &str = "SNAFU_RAW_ERROR_MESSAGES";

#[cfg(feature = "std")]
fn trace_cleaning_enabled() -> bool {
use crate::once_bool::OnceBool;
use std::env;

static DISABLED: OnceBool = OnceBool::new();
!DISABLED.get(|| env::var_os(SNAFU_RAW_ERROR_MESSAGES).map_or(false, |v| v == "1"))
}

#[doc(hidden)]
pub trait __InternalExtractErrorType {
type Err;
Expand Down
76 changes: 76 additions & 0 deletions tests/report.rs
Expand Up @@ -11,6 +11,17 @@ macro_rules! assert_contains {
};
}

macro_rules! assert_not_contains {
(needle: $needle:expr, haystack: $haystack:expr) => {
assert!(
!$haystack.contains($needle),
"Expected {:?} to not include {:?}",
$haystack,
$needle,
)
};
}

#[test]
fn includes_the_error_display_text() {
#[derive(Debug, Snafu)]
Expand Down Expand Up @@ -44,6 +55,71 @@ fn includes_the_source_display_text() {
assert_contains!(needle: expected, haystack: msg);
}

#[test]
fn reduces_duplication_of_the_source_display_text() {
// Including the source in the Display message is discouraged but
// quite common.

#[derive(Debug, Snafu)]
#[snafu(display("Level 0"))]
struct Level0Error;

#[derive(Debug, Snafu)]
#[snafu(display("Level 1: {source}"))]
struct Level1Error {
source: Level0Error,
}

#[derive(Debug, Snafu)]
#[snafu(display("Level 2: {source}"))]
struct Level2Error {
source: Level1Error,
}

let e = Level2Snafu.into_error(Level1Snafu.into_error(Level0Error));
let raw_msg = e.to_string();

let expected = "Level 2: Level 1";
assert_contains!(needle: expected, haystack: raw_msg);

let r = Report::from_error(e);
let msg = r.to_string();

assert_not_contains!(needle: expected, haystack: msg);
}

#[test]
fn removes_complete_duplication_in_the_source_display_text() {
// Including **only** the source in the Display message is also
// discouraged but occurs.

#[derive(Debug, Snafu)]
#[snafu(display("Level 0"))]
struct Level0Error;

#[derive(Debug, Snafu)]
#[snafu(display("{source}"))]
struct Level1Error {
source: Level0Error,
}

#[derive(Debug, Snafu)]
#[snafu(display("{source}"))]
struct Level2Error {
source: Level1Error,
}

let e = Level2Snafu.into_error(Level1Snafu.into_error(Level0Error));
let raw_msg = e.to_string();

assert_contains!(needle: "Level 0", haystack: raw_msg);

let r = Report::from_error(e);
let msg = r.to_string();

assert_not_contains!(needle: "Caused by", haystack: msg);
}

#[test]
fn debug_and_display_are_the_same() {
#[derive(Debug, Snafu)]
Expand Down

0 comments on commit ed78681

Please sign in to comment.