From 2ea2b1f4fb50965041d0c47e6605c64ea967cfe7 Mon Sep 17 00:00:00 2001
From: Alex Orlenko <zxteam@protonmail.com>
Date: Sun, 7 Nov 2021 21:21:50 +0000
Subject: [PATCH] Refactor `Error::CallbackError` reporting and include source
 to fmt::Display implementation. This fixes #71.

---
 src/error.rs | 30 +++++++++++++++++++++++++++---
 src/util.rs  | 28 ----------------------------
 2 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index 47915af..760c49b 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -247,8 +247,28 @@ impl fmt::Display for Error {
             Error::MismatchedRegistryKey => {
                 write!(fmt, "RegistryKey used from different Lua state")
             }
-            Error::CallbackError { ref traceback, .. } => {
-                write!(fmt, "callback error\n{}", traceback)
+            Error::CallbackError { ref cause, ref traceback } => {
+                writeln!(fmt, "callback error")?;
+                // Trace errors down to the root
+                let (mut cause, mut full_traceback) = (cause, None);
+                while let Error::CallbackError { cause: ref cause2, traceback: ref traceback2 } = **cause {
+                    cause = cause2;
+                    full_traceback = Some(traceback2);
+                }
+                if let Some(full_traceback) = full_traceback {
+                    let traceback = traceback.trim_start_matches("stack traceback:");
+                    let traceback = traceback.trim_start().trim_end();
+                    // Try to find local traceback within the full traceback
+                    if let Some(pos) = full_traceback.find(traceback) {
+                        write!(fmt, "{}", &full_traceback[..pos])?;
+                        writeln!(fmt, ">{}", &full_traceback[pos..].trim_end())?;
+                    } else {
+                        writeln!(fmt, "{}", full_traceback.trim_end())?;
+                    }
+                } else {
+                    writeln!(fmt, "{}", traceback.trim_end())?;
+                }
+                write!(fmt, "caused by: {}", cause)
             }
             Error::PreviouslyResumedPanic => {
                 write!(fmt, "previously resumed panic returned again")
@@ -269,7 +289,11 @@ impl fmt::Display for Error {
 impl StdError for Error {
     fn source(&self) -> Option<&(dyn StdError + 'static)> {
         match *self {
-            Error::CallbackError { ref cause, .. } => Some(cause.as_ref()),
+            // An error type with a source error should either return that error via source or
+            // include that source's error message in its own Display output, but never both.
+            // https://blog.rust-lang.org/inside-rust/2021/07/01/What-the-error-handling-project-group-is-working-towards.html
+            // Given that we include source to fmt::Display implementation for `CallbackError`, this call returns nothing.
+            Error::CallbackError { .. } => None,
             Error::ExternalError(ref err) => err.source(),
             _ => None,
         }
diff --git a/src/util.rs b/src/util.rs
index c028758..e995cfb 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -1,5 +1,4 @@
 use std::any::{Any, TypeId};
-use std::error::Error as StdError;
 use std::fmt::Write;
 use std::os::raw::{c_char, c_int, c_void};
 use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};
@@ -736,32 +735,6 @@ pub unsafe fn init_error_registry(state: *mut ffi::lua_State) -> Result<()> {
                     // be possible to make this consume arbitrary amounts of memory (for example, some
                     // kind of recursive error structure?)
                     let _ = write!(&mut (*err_buf), "{}", error);
-                    // Find first two sources that caused the error
-                    let mut source1 = error.source();
-                    let mut source0 = source1.and_then(|s| s.source());
-                    while let Some(source) = source0.and_then(|s| s.source()) {
-                        source1 = source0;
-                        source0 = Some(source);
-                    }
-                    match (source1, source0) {
-                        (_, Some(error0))
-                            if error0.to_string().contains("\nstack traceback:\n") =>
-                        {
-                            let _ = write!(&mut (*err_buf), "\ncaused by: {}", error0);
-                        }
-                        (Some(error1), Some(error0)) => {
-                            let _ = write!(&mut (*err_buf), "\ncaused by: {}", error0);
-                            let s = error1.to_string();
-                            if let Some(traceback) = s.split_once("\nstack traceback:\n") {
-                                let _ =
-                                    write!(&mut (*err_buf), "\nstack traceback:\n{}", traceback.1);
-                            }
-                        }
-                        (Some(error1), None) => {
-                            let _ = write!(&mut (*err_buf), "\ncaused by: {}", error1);
-                        }
-                        _ => {}
-                    }
                     Ok(err_buf)
                 }
                 Some(WrappedFailure::Panic(Some(ref panic))) => {
@@ -805,7 +778,6 @@ pub unsafe fn init_error_registry(state: *mut ffi::lua_State) -> Result<()> {
     // Create destructed userdata metatable
 
     unsafe extern "C" fn destructed_error(state: *mut ffi::lua_State) -> c_int {
-        // TODO: Consider changing error to UserDataDestructed in v0.7
         callback_error(state, |_| Err(Error::CallbackDestructed))
     }