Merge pull request #6 from risingwavelabs/bz/show-backtrace-only-if-c…

…aptured

src/report.rs

@@ -15,7 +15,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{backtrace::Backtrace, fmt};
+use std::{
+    backtrace::{Backtrace, BacktraceStatus},
+    fmt,
+};
 
 /// Extension trait for [`Error`] that provides a [`Report`] which formats
 /// the error and its sources in a cleaned-up way.
@@ -184,14 +187,24 @@ impl<'a> fmt::Display for Report<'a> {
 
 impl<'a> fmt::Debug for Report<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        // Hack for testing purposes.
+        // Read the env var could be slow but we short-circuit it in release mode,
+        // so this should be optimized out in production.
+        let force_show_backtrace = cfg!(debug_assertions)
+            && std::env::var("THISERROR_EXT_TEST_SHOW_USELESS_BACKTRACE").is_ok();
+
         self.cleaned_error_trace(f, f.alternate())?;
 
         if let Some(bt) = std::error::request_ref::<Backtrace>(self.0) {
-            // Print a newline if we're not in alternate mode.
-            if !f.alternate() {
-                writeln!(f)?;
+            // If the backtrace is disabled or unsupported, behave as if there's no backtrace.
+            if bt.status() == BacktraceStatus::Captured || force_show_backtrace {
+                // The alternate mode contains a trailing newline while non-alternate
+                // mode does not. So we need to add a newline before the backtrace.
+                if !f.alternate() {
+                    writeln!(f)?;
+                }
+                writeln!(f, "\nBacktrace:\n{}", bt)?;
             }
-            writeln!(f, "\nBacktrace:\n{}", bt)?;
         }
 
         Ok(())

tests/report.rs

@@ -95,7 +95,7 @@ fn test_report_display_alternate_single_source() {
 
 // Show that there's extra backtrace information compared to `Display`.
 // Backtrace is intentionally disabled to make the test deterministic.
-#[sealed_test(env = [("RUST_BACKTRACE", "0")])]
+#[sealed_test(env = [("RUST_BACKTRACE", "0"), ("THISERROR_EXT_TEST_SHOW_USELESS_BACKTRACE", "1")])]
 fn test_report_debug() {
     let expect = expect![[r#"
         outer error: middle error: inner error
@@ -115,7 +115,7 @@ fn test_report_debug_no_backtrace() {
 
 // Show that there's extra backtrace information compared to `Display`.
 // Backtrace is intentionally disabled to make the test deterministic.
-#[sealed_test(env = [("RUST_BACKTRACE", "0")])]
+#[sealed_test(env = [("RUST_BACKTRACE", "0"), ("THISERROR_EXT_TEST_SHOW_USELESS_BACKTRACE", "1")])]
 fn test_report_debug_alternate() {
     let expect = expect![[r#"
         outer error
@@ -142,3 +142,17 @@ fn test_report_debug_alternate_no_backtrace() {
     "#]];
     expect.assert_eq(&format!("{:#?}", outer(false).unwrap_err().as_report()));
 }
+
+// If there's disabled backtrace, the behavior should be exactly the same as `Display` too.
+// Note that `THISERROR_EXT_TEST_SHOW_USELESS_BACKTRACE` is not set so this mimics the user's environment.
+#[sealed_test(env = [("RUST_BACKTRACE", "0")])]
+fn test_report_debug_alternate_disabled_backtrace() {
+    let expect = expect![[r#"
+        outer error
+
+        Caused by these errors (recent errors listed first):
+          1: middle error
+          2: inner error
+    "#]];
+    expect.assert_eq(&format!("{:#?}", outer(true).unwrap_err().as_report()));
+}