properly re-catch mlua errors

Author: makspll

crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs

@@ -304,6 +304,10 @@ impl ReflectReferencePrinter {
     }
 }
 
+/// Alais for [`DisplayWithWorldAndDummy`] + [`std::fmt::Display`], ideally display should warn that it's not the full representation.
+pub trait DisplayWithWorldAndDummy: DisplayWithWorld + std::fmt::Display {}
+impl<T: DisplayWithWorld + std::fmt::Display> DisplayWithWorldAndDummy for T {}
+
 /// For types which can't be pretty printed without world access.
 /// Implementors should try to print the best value they can, and never panick.
 pub trait DisplayWithWorld: std::fmt::Debug {
@@ -330,6 +334,8 @@ macro_rules! impl_dummy_display (
     };
 );
 
+impl_dummy_display!(ReflectReference);
+
 impl DisplayWithWorld for ReflectReference {
     fn display_with_world(&self, world: WorldGuard) -> String {
         ReflectReferencePrinter::new(self.clone()).pretty_print(world)
@@ -340,6 +346,8 @@ impl DisplayWithWorld for ReflectReference {
     }
 }
 
+impl_dummy_display!(ReflectBaseType);
+
 impl DisplayWithWorld for ReflectBaseType {
     fn display_with_world(&self, world: WorldGuard) -> String {
         let mut string = String::new();
@@ -371,6 +379,8 @@ impl DisplayWithWorld for TypeId {
     }
 }
 
+impl_dummy_display!(ScriptValue);
+
 impl DisplayWithWorld for ScriptValue {
     fn display_with_world(&self, world: WorldGuard) -> String {
         match self {

crates/bevy_mod_scripting_core/src/error.rs

@@ -18,34 +18,22 @@ use thiserror::Error;
 
 use crate::{
     bindings::{
-        pretty_print::DisplayWithWorld, ReflectAllocationId, ReflectBase, ReflectBaseType,
-        ReflectReference,
+        pretty_print::{DisplayWithWorld, DisplayWithWorldAndDummy},
+        ReflectAllocationId, ReflectBase, ReflectBaseType, ReflectReference,
     },
     impl_dummy_display,
     prelude::ScriptValue,
 };
 
 pub type ScriptResult<T> = Result<T, ScriptError>;
 
-#[derive(Error, Debug)]
-pub struct ScriptErrorWrapper(ScriptError);
-
-impl std::fmt::Display for ScriptErrorWrapper {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0)
-    }
-}
-
-impl From<ScriptError> for Box<dyn std::error::Error + Send + Sync + 'static> {
-    fn from(val: ScriptError) -> Self {
-        ScriptErrorWrapper(val).into()
-    }
-}
 /// An error with an optional script Context
 #[derive(Debug, Clone, PartialEq, Reflect)]
 #[reflect(opaque)]
 pub struct ScriptError(pub Arc<ScriptErrorInner>);
 
+impl std::error::Error for ScriptError {}
+
 impl Deref for ScriptError {
     type Target = ScriptErrorInner;
 
@@ -65,7 +53,7 @@ pub struct ScriptErrorInner {
 #[derive(Debug, Clone)]
 pub enum ErrorKind {
     Display(Arc<dyn std::error::Error + Send + Sync>),
-    WithWorld(Arc<dyn DisplayWithWorld + Send + Sync>),
+    WithWorld(Arc<dyn DisplayWithWorldAndDummy + Send + Sync>),
 }
 
 impl DisplayWithWorld for ErrorKind {
@@ -81,7 +69,7 @@ impl Display for ErrorKind {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             ErrorKind::Display(e) => write!(f, "{}", e),
-            ErrorKind::WithWorld(e) => write!(f, "{:?}", e),
+            ErrorKind::WithWorld(e) => write!(f, "{}", e),
         }
     }
 }
@@ -92,31 +80,26 @@ impl PartialEq for ScriptErrorInner {
     }
 }
 
-// impl<T: std::error::Error + Send + Sync + 'static> From<T> for ErrorKind {
-//     fn from(value: T) -> Self {
-//         ErrorKind::Display(Arc::new(value))
-//     }
-// }
-
-// impl<T: DisplayWithWorld + Send + Sync + 'static> From<T> for ErrorKind {
-//     fn from(val: Box<dyn DisplayWithWorld + Send + Sync>) -> Self {
-//         ErrorKind::WithWorld(Arc::from(val))
-//     }
-// }
-
 impl ScriptError {
     #[cfg(feature = "mlua_impls")]
     /// Destructures mlua error into a script error, taking care to preserve as much information as possible
     pub fn from_mlua_error(error: mlua::Error) -> Self {
         match error {
-            mlua::Error::ExternalError(inner) => {
-                if let Some(script_error) = inner.downcast_ref::<InteropError>() {
-                    script_error.clone().into()
+            mlua::Error::CallbackError { traceback, cause }
+                if matches!(cause.as_ref(), mlua::Error::ExternalError(_)) =>
+            {
+                let inner = cause.deref().clone();
+                Self::from_mlua_error(inner).with_context(traceback)
+            }
+            e => {
+                if let Some(inner) = e.downcast_ref::<InteropError>() {
+                    Self::new(inner.clone())
+                } else if let Some(inner) = e.downcast_ref::<ScriptError>() {
+                    inner.clone()
                 } else {
-                    Self::new_external(inner)
+                    Self::new_external(e)
                 }
             }
-            e => Self::new_external(e),
         }
     }
 
@@ -128,7 +111,7 @@ impl ScriptError {
         }))
     }
 
-    pub fn new(reason: impl DisplayWithWorld + Send + Sync + 'static) -> Self {
+    pub fn new(reason: impl DisplayWithWorldAndDummy + Send + Sync + 'static) -> Self {
         Self(Arc::new(ScriptErrorInner {
             script: None,
             reason: ErrorKind::WithWorld(Arc::new(reason)),
@@ -144,6 +127,14 @@ impl ScriptError {
         }))
     }
 
+    pub fn with_script<S: ToString>(self, script: S) -> Self {
+        Self(Arc::new(ScriptErrorInner {
+            script: Some(script.to_string()),
+            context: self.0.context.clone(),
+            reason: self.0.reason.clone(),
+        }))
+    }
+
     pub fn with_appended_context<S: ToString>(self, context: S) -> Self {
         Self(Arc::new(ScriptErrorInner {
             script: self.0.script.clone(),
@@ -153,26 +144,16 @@ impl ScriptError {
     }
 }
 
-// impl<T: std::error::Error + Send + Sync + 'static> From<T> for ScriptError {
-//     fn from(value: T) -> Self {
-//         let error_kind = ErrorKind::from(value);
-//         Self::new_external(error_kind)
-//     }
-// }
-
-// impl<T: DisplayWithWorld + Send + Sync + 'static> From<T> for ScriptError {
-//     fn from(value: T) -> Self {
-//         let error_kind = ErrorKind::WithWorld(Arc::new(value));
-//         Self::new_error(error_kind)
-//     }
-// }
-
 impl std::fmt::Display for ScriptError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         if let Some(script) = &self.0.script {
-            write!(f, "error in script `{}`: {}", script, self.0.reason)
+            write!(
+                f,
+                "error in script `{}`: {}.\n{}",
+                script, self.0.reason, self.0.context
+            )
         } else {
-            write!(f, "error: {}", self.0.reason)
+            write!(f, "error: {}.\n{}", self.0.reason, self.0.context)
         }
     }
 }
@@ -181,12 +162,17 @@ impl DisplayWithWorld for ScriptError {
     fn display_with_world(&self, world: crate::bindings::WorldGuard) -> String {
         if let Some(script) = &self.0.script {
             format!(
-                "error in script `{}`: {}",
+                "error in script `{}`: {}.\n{}",
                 script,
-                self.0.reason.display_with_world(world)
+                self.0.reason.display_with_world(world),
+                self.0.context
             )
         } else {
-            format!("error: {}", self.0.reason.display_with_world(world))
+            format!(
+                "error: {}.\n{}",
+                self.0.reason.display_with_world(world),
+                self.0.context
+            )
         }
     }
 }
@@ -350,8 +336,10 @@ impl InteropError {
     }
 }
 
-#[derive(Debug)]
+impl_dummy_display!(InteropErrorInner);
+
 /// For errors to do with reflection, type conversions or other interop issues
+#[derive(Debug)]
 pub enum InteropErrorInner {
     StaleWorldAccess,
     MissingWorld,

crates/bevy_mod_scripting_core/src/systems.rs

@@ -187,7 +187,8 @@ pub fn event_handler<L: IntoCallbackLabel, A: Args, C: Context, R: Runtime>(
                     world,
                 )
                 .map_err(|e| {
-                    e.with_appended_context(format!("Script: {}, Entity: {:?}", script.id, entity))
+                    // println!("{e:?}");
+                    e.with_script(script.id.clone())
                 });
 
                 push_err_and_continue!(errors, handler_result)

crates/languages/bevy_mod_scripting_lua/src/assets.rs

@@ -4,7 +4,6 @@
 //     utils::BoxedFuture,
 // };
 
-
 // use anyhow::Error;
 
 // #[derive(Asset, TypePath, Debug)]
@@ -146,7 +145,6 @@
 //         Ok(LuaFile { bytes })
 //     }
 
-
 //     #[cfg(feature = "teal")]
 //     fn extensions(&self) -> &[&str] {
 //         &["lua", "tl"]

crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs

@@ -22,7 +22,7 @@ use bevy_mod_scripting_core::{
         ReflectAllocator, ReflectRefIter, ReflectReference, ReflectionPathExt, TypeIdSource,
         WorldCallbackAccess,
     },
-    error::{FunctionError, ScriptError, ScriptResult, InteropError},
+    error::{InteropError, ScriptError, ScriptResult},
     reflection_extensions::{PartialReflectExt, TypeIdExtensions},
     Either,
 };

crates/languages/bevy_mod_scripting_lua/src/bindings/world.rs

@@ -5,7 +5,7 @@ use bevy::prelude::{AppFunctionRegistry, Entity, World};
 
 use bevy_mod_scripting_core::bindings::function::CallableWithAccess;
 use bevy_mod_scripting_core::bindings::WorldGuard;
-use bevy_mod_scripting_core::error::{FunctionError, InteropError};
+use bevy_mod_scripting_core::error::InteropError;
 use bevy_mod_scripting_core::{
     bindings::{ReflectReference, ScriptTypeRegistration, WorldAccessGuard, WorldCallbackAccess},
     error::ScriptError,

crates/languages/bevy_mod_scripting_lua/src/lib.rs

@@ -69,11 +69,17 @@ impl<A: LuaEventArg> Plugin for LuaScriptingPlugin<A> {
             //     let reflect_reference = ReflectReference::new_allocated(entity, &mut allocator);
             //     <Entity as LuaProxied>::Proxy::from(reflect_reference)
             // });
-            context.globals().set(
-                "entity",
-                LuaReflectReference(<Entity>::allocate(Box::new(entity), world)),
-            )?;
-            context.globals().set("script_id", script_id.clone())?;
+            context
+                .globals()
+                .set(
+                    "entity",
+                    LuaReflectReference(<Entity>::allocate(Box::new(entity), world)),
+                )
+                .map_err(ScriptError::from_mlua_error)?;
+            context
+                .globals()
+                .set("script_id", script_id.clone())
+                .map_err(ScriptError::from_mlua_error)?;
             // context.globals().set("entity", lua_entity)?;
             Ok(())
         });
@@ -110,7 +116,10 @@ pub fn lua_context_load(
             .iter()
             .try_for_each(|init| init(script_id, Entity::from_raw(0), context))?;
 
-        context.load(content).exec()?;
+        context
+            .load(content)
+            .exec()
+            .map_err(ScriptError::from_mlua_error)?;
         Ok(())
     })?;
 
@@ -159,7 +168,9 @@ pub fn lua_handler<A: Args + for<'l> IntoLuaMulti<'l>>(
             Err(_) => return Ok(()),
         };
 
-        handler.call::<_, ()>(args)?;
+        handler
+            .call::<_, ()>(args)
+            .map_err(ScriptError::from_mlua_error)?;
         Ok(())
     })
 }
@@ -173,7 +184,8 @@ pub fn with_world<F: FnOnce(&mut Lua) -> Result<(), ScriptError>>(
     WorldCallbackAccess::with_callback_access(world, |guard| {
         context
             .globals()
-            .set("world", LuaReflectReference(ReflectReference::new_world()))?;
+            .set("world", LuaReflectReference(ReflectReference::new_world()))
+            .map_err(ScriptError::from_mlua_error)?;
         context.set_app_data(guard.clone());
         f(context)
     })