Remove redundant stack trace information in error formatter

crates/lune-utils/src/fmt/error/components.rs

@@ -124,7 +124,7 @@ impl From<LuaError> for ErrorComponents {
         }
 
         // We will then try to extract any stack trace
-        let trace = if let LuaError::CallbackError {
+        let mut trace = if let LuaError::CallbackError {
             ref traceback,
             ref cause,
         } = *error
@@ -147,6 +147,45 @@ impl From<LuaError> for ErrorComponents {
             None
         };
 
+        // Sometimes, we can get duplicate stack trace lines that only
+        // mention "[C]", without a function name or path, and these can
+        // be safely ignored / removed if the following line has more info
+        if let Some(trace) = &mut trace {
+            let lines = trace.lines_mut();
+            loop {
+                let first_is_c_and_empty = lines
+                    .first()
+                    .is_some_and(|line| line.source().is_c() && line.is_empty());
+                let second_is_c_and_nonempty = lines
+                    .get(1)
+                    .is_some_and(|line| line.source().is_c() && !line.is_empty());
+                if first_is_c_and_empty && second_is_c_and_nonempty {
+                    lines.remove(0);
+                } else {
+                    break;
+                }
+            }
+        }
+
+        // Finally, we do some light postprocessing to remove duplicate
+        // information, such as the location prefix in the error message
+        if let Some(message) = messages.last_mut() {
+            if let Some(line) = trace
+                .iter()
+                .flat_map(StackTrace::lines)
+                .find(|line| line.source().is_lua())
+            {
+                let location_prefix = format!(
+                    "[string \"{}\"]:{}:",
+                    line.path().unwrap(),
+                    line.line_number().unwrap()
+                );
+                if message.starts_with(&location_prefix) {
+                    *message = message[location_prefix.len()..].trim().to_string();
+                }
+            }
+        }
+
         ErrorComponents { messages, trace }
     }
 }

crates/lune-utils/src/fmt/error/stack_trace.rs

@@ -39,6 +39,24 @@ pub enum StackTraceSource {
     Lua,
 }
 
+impl StackTraceSource {
+    /**
+        Returns `true` if the error originated from a C / Rust function, `false` otherwise.
+    */
+    #[must_use]
+    pub const fn is_c(self) -> bool {
+        matches!(self, Self::C)
+    }
+
+    /**
+        Returns `true` if the error originated from a Lua (user) function, `false` otherwise.
+    */
+    #[must_use]
+    pub const fn is_lua(self) -> bool {
+        matches!(self, Self::Lua)
+    }
+}
+
 /**
     Stack trace line parsed from a [`LuaError`].
 */
@@ -82,6 +100,20 @@ impl StackTraceLine {
     pub fn function_name(&self) -> Option<&str> {
         self.function_name.as_deref()
     }
+
+    /**
+        Returns `true` if the stack trace line contains no "useful" information, `false` otherwise.
+
+        Useful information is determined as one of:
+
+        - A path
+        - A line number
+        - A function name
+    */
+    #[must_use]
+    pub const fn is_empty(&self) -> bool {
+        self.path.is_none() && self.line_number.is_none() && self.function_name.is_none()
+    }
 }
 
 impl FromStr for StackTraceLine {
@@ -145,6 +177,14 @@ impl StackTrace {
     pub fn lines(&self) -> &[StackTraceLine] {
         &self.lines
     }
+
+    /**
+        Returns the individual stack trace lines, mutably.
+    */
+    #[must_use]
+    pub fn lines_mut(&mut self) -> &mut Vec<StackTraceLine> {
+        &mut self.lines
+    }
 }
 
 impl FromStr for StackTrace {

crates/lune-utils/src/fmt/error/tests.rs

@@ -2,7 +2,7 @@ use mlua::prelude::*;
 
 use crate::fmt::ErrorComponents;
 
-fn new_lua_result() -> LuaResult<()> {
+fn new_lua_runtime_error() -> LuaResult<()> {
     let lua = Lua::new();
 
     lua.globals()
@@ -17,13 +17,34 @@ fn new_lua_result() -> LuaResult<()> {
     lua.load("f()").set_name("chunk_name").eval()
 }
 
+fn new_lua_script_error() -> LuaResult<()> {
+    let lua = Lua::new();
+
+    lua.load(
+        "local function inner()\
+        \n    error(\"oh no, a script error\")\
+        \nend\
+        \n\
+        \nlocal function outer()\
+        \n    inner()\
+        \nend\
+        \n\
+        \nouter()\
+        ",
+    )
+    .set_name("chunk_name")
+    .eval()
+}
+
 // Tests for error context stack
 mod context {
     use super::*;
 
     #[test]
     fn preserves_original() {
-        let lua_error = new_lua_result().context("additional context").unwrap_err();
+        let lua_error = new_lua_runtime_error()
+            .context("additional context")
+            .unwrap_err();
         let components = ErrorComponents::from(lua_error);
 
         assert_eq!(components.messages()[0], "additional context");
@@ -34,7 +55,7 @@ mod context {
     fn preserves_levels() {
         // NOTE: The behavior in mlua is to preserve a single level of context
         // and not all levels (context gets replaced on each call to `context`)
-        let lua_error = new_lua_result()
+        let lua_error = new_lua_runtime_error()
             .context("level 1")
             .context("level 2")
             .context("level 3")
@@ -54,7 +75,7 @@ mod error_components {
 
     #[test]
     fn message() {
-        let lua_error = new_lua_result().unwrap_err();
+        let lua_error = new_lua_runtime_error().unwrap_err();
         let components = ErrorComponents::from(lua_error);
 
         assert_eq!(components.messages()[0], "oh no, a runtime error");
@@ -62,7 +83,7 @@ mod error_components {
 
     #[test]
     fn stack_begin_end() {
-        let lua_error = new_lua_result().unwrap_err();
+        let lua_error = new_lua_runtime_error().unwrap_err();
         let formatted = format!("{}", ErrorComponents::from(lua_error));
 
         assert!(formatted.contains("Stack Begin"));
@@ -71,7 +92,7 @@ mod error_components {
 
     #[test]
     fn stack_lines() {
-        let lua_error = new_lua_result().unwrap_err();
+        let lua_error = new_lua_runtime_error().unwrap_err();
         let components = ErrorComponents::from(lua_error);
 
         let mut lines = components.trace().unwrap().lines().iter();
@@ -83,3 +104,47 @@ mod error_components {
         assert_eq!(line_2, "Script 'chunk_name', Line 1");
     }
 }
+
+// Tests for general formatting
+mod general {
+    use super::*;
+
+    #[test]
+    fn message_does_not_contain_location() {
+        let lua_error = new_lua_script_error().unwrap_err();
+
+        let components = ErrorComponents::from(lua_error);
+        let trace = components.trace().unwrap();
+
+        let first_message = components.messages().first().unwrap();
+        let first_lua_stack_line = trace
+            .lines()
+            .iter()
+            .find(|line| line.source().is_lua())
+            .unwrap();
+
+        let location_prefix = format!(
+            "[string \"{}\"]:{}:",
+            first_lua_stack_line.path().unwrap(),
+            first_lua_stack_line.line_number().unwrap()
+        );
+
+        assert!(!first_message.starts_with(&location_prefix));
+    }
+
+    #[test]
+    fn no_redundant_c_mentions() {
+        let lua_error = new_lua_script_error().unwrap_err();
+
+        let components = ErrorComponents::from(lua_error);
+        let trace = components.trace().unwrap();
+
+        let c_stack_lines = trace
+            .lines()
+            .iter()
+            .filter(|line| line.source().is_c())
+            .collect::<Vec<_>>();
+
+        assert_eq!(c_stack_lines.len(), 1); // Just the "error" call
+    }
+}