From 214da816bb599a3abd61e8c6742d9d87d613524e Mon Sep 17 00:00:00 2001
From: Jake Goulding <jake.goulding@gmail.com>
Date: Wed, 19 Oct 2022 14:41:42 -0400
Subject: [PATCH] Automatically detect and remove nested error messages in
 Report output

Closes #363
---
 src/report.rs   | 90 +++++++++++++++++++++++++++++++++++++++++++++----
 tests/report.rs | 43 +++++++++++++++++++++++
 2 files changed, 127 insertions(+), 6 deletions(-)

diff --git a/src/report.rs b/src/report.rs
index 6fe2962b..4cb54965 100644
--- a/src/report.rs
+++ b/src/report.rs
@@ -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);
@@ -236,19 +265,68 @@ 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> {
+        let original_messages: Vec<_> = ChainCompat::new(self.0).map(ToString::to_string).collect();
+
+        let message_pairs = original_messages
+            .iter()
+            .zip(original_messages.iter().skip(1));
+        let cleaned_messages = message_pairs.map(|(a, b)| {
+            let cleaned = a.trim_end_matches(b).trim_end().trim_end_matches(":");
+            (cleaned, a != cleaned)
+        });
+        let mut cleaned_messages =
+            cleaned_messages.chain(original_messages.last().map(|msg| (&**msg, false)));
+
+        let (head, cleaned) = match cleaned_messages.next() {
+            Some(v) => v,
+            None => return Ok(()),
+        };
+
+        let mut any_cleaned = cleaned;
+        let marker = |b| if b { " *" } else { "" };
+
+        writeln!(f, "{}{}", head, marker(cleaned))?;
+
+        let plurality = original_messages.len();
+
+        match plurality {
+            0 => {}
+            1 => writeln!(f, "\nCaused by this error:")?,
+            _ => writeln!(f, "\nCaused by these errors (recent errors listed first):")?,
+        }
+
+        for (i, (source, cleaned)) in cleaned_messages.enumerate() {
+            // Let's use 1-based indexing for presentation
+            let i = i + 1;
+            writeln!(f, "{:3}: {}{}", i, source, marker(cleaned))?;
+            any_cleaned = any_cleaned || cleaned;
+        }
+
+        if any_cleaned {
+            writeln!(f, "\nNOTE: Some redundant information has been removed from the lines marked with *. 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;
diff --git a/tests/report.rs b/tests/report.rs
index 476fc624..966502f0 100644
--- a/tests/report.rs
+++ b/tests/report.rs
@@ -43,6 +43,49 @@ fn includes_the_source_display_text() {
     );
 }
 
+#[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!(
+        raw_msg.contains(expected),
+        "Expected {:?} to include {:?}",
+        raw_msg,
+        expected,
+    );
+
+    let r = Report::from_error(e);
+    let msg = r.to_string();
+
+    assert!(
+        !msg.contains(expected),
+        "Expected {:?} to not include {:?}",
+        msg,
+        expected,
+    );
+}
+
 #[test]
 fn debug_and_display_are_the_same() {
     #[derive(Debug, Snafu)]