More user-friendly error message on userdata mismatch

mlua-rs · Sep 23, 2024 · 8274b5f · 8274b5f
1 parent 762e677
commit 8274b5f
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 19 deletions.
diff --git a/src/state/raw.rs b/src/state/raw.rs
@@ -1004,8 +1004,18 @@ impl RawLua {
     }
 
     // Same as `get_userdata_ref_type_id` but assumes the userdata is already on the stack.
-    pub(crate) unsafe fn get_userdata_type_id(&self, idx: c_int) -> Result<Option<TypeId>> {
-        self.get_userdata_type_id_inner(self.state(), idx)
+    pub(crate) unsafe fn get_userdata_type_id<T>(&self, idx: c_int) -> Result<Option<TypeId>> {
+        match self.get_userdata_type_id_inner(self.state(), idx) {
+            Ok(type_id) => Ok(type_id),
+            Err(Error::UserDataTypeMismatch) if ffi::lua_type(self.state(), idx) != ffi::LUA_TUSERDATA => {
+                // Report `FromLuaConversionError` instead
+                let idx_type_name = CStr::from_ptr(ffi::luaL_typename(self.state(), idx));
+                let idx_type_name = idx_type_name.to_str().unwrap();
+                let message = format!("expected userdata of type '{}'", short_type_name::<T>());
+                Err(Error::from_lua_conversion(idx_type_name, "userdata", message))
+            }
+            Err(err) => Err(err),
+        }
     }
 
     unsafe fn get_userdata_type_id_inner(

diff --git a/src/userdata/cell.rs b/src/userdata/cell.rs
@@ -195,7 +195,7 @@ impl<T: 'static> FromLua for UserDataRef<T> {
     }
 
     unsafe fn from_stack(idx: c_int, lua: &RawLua) -> Result<Self> {
-        let type_id = lua.get_userdata_type_id(idx)?;
+        let type_id = lua.get_userdata_type_id::<T>(idx)?;
         match type_id {
             Some(type_id) if type_id == TypeId::of::<T>() => {
                 (*get_userdata::<UserDataStorage<T>>(lua.state(), idx)).try_borrow_owned()
@@ -263,7 +263,7 @@ impl<T: 'static> FromLua for UserDataRefMut<T> {
     }
 
     unsafe fn from_stack(idx: c_int, lua: &RawLua) -> Result<Self> {
-        let type_id = lua.get_userdata_type_id(idx)?;
+        let type_id = lua.get_userdata_type_id::<T>(idx)?;
         match type_id {
             Some(type_id) if type_id == TypeId::of::<T>() => {
                 (*get_userdata::<UserDataStorage<T>>(lua.state(), idx)).try_borrow_owned_mut()

diff --git a/src/userdata/registry.rs b/src/userdata/registry.rs
@@ -121,7 +121,7 @@ impl<T> UserDataRegistry<T> {
             match target_type_id {
                 // This branch is for `'static` userdata that share type metatable
                 UserDataTypeId::Shared(target_type_id) => {
-                    match try_self_arg!(rawlua.get_userdata_type_id(self_index)) {
+                    match try_self_arg!(rawlua.get_userdata_type_id::<T>(self_index)) {
                         Some(self_type_id) if self_type_id == target_type_id => {
                             let ud = get_userdata::<UserDataStorage<T>>(state, self_index);
                             try_self_arg!((*ud).try_borrow_scoped(|ud| {
@@ -175,7 +175,7 @@ impl<T> UserDataRegistry<T> {
             match target_type_id {
                 // This branch is for `'static` userdata that share type metatable
                 UserDataTypeId::Shared(target_type_id) => {
-                    match try_self_arg!(rawlua.get_userdata_type_id(self_index)) {
+                    match try_self_arg!(rawlua.get_userdata_type_id::<T>(self_index)) {
                         Some(self_type_id) if self_type_id == target_type_id => {
                             let ud = get_userdata::<UserDataStorage<T>>(state, self_index);
                             try_self_arg!((*ud).try_borrow_scoped_mut(|ud| {

diff --git a/tests/userdata.rs b/tests/userdata.rs
@@ -206,8 +206,8 @@ fn test_metamethods() -> Result<()> {
     Ok(())
 }
 
-#[test]
 #[cfg(feature = "lua54")]
+#[test]
 fn test_metamethod_close() -> Result<()> {
     #[derive(Clone)]
     struct MyUserData(Arc<AtomicI64>);
@@ -791,18 +791,27 @@ fn test_userdata_method_errors() -> Result<()> {
     let lua = Lua::new();
 
     let ud = lua.create_userdata(MyUserData(123))?;
-    let res = ud.call_function::<()>("get_value", ());
-    let Err(Error::CallbackError { cause, .. }) = res else {
-        panic!("expected CallbackError, got {res:?}");
-    };
-    assert!(matches!(
-        &*cause,
-        Error::BadArgument {
-            to,
-            name,
-            ..
-        } if to.as_deref() == Some("MyUserData.get_value") && name.as_deref() == Some("self")
-    ));
+    let res = ud.call_function::<()>("get_value", "not a userdata");
+    match res {
+        Err(Error::CallbackError { cause, .. }) => match cause.as_ref() {
+            Error::BadArgument {
+                to,
+                name,
+                cause: cause2,
+                ..
+            } => {
+                assert_eq!(to.as_deref(), Some("MyUserData.get_value"));
+                assert_eq!(name.as_deref(), Some("self"));
+                println!("{}", cause2.to_string());
+                assert_eq!(
+                    cause2.to_string(),
+                    "error converting Lua string to userdata (expected userdata of type 'MyUserData')"
+                );
+            }
+            err => panic!("expected BadArgument, got {err:?}"),
+        },
+        r => panic!("expected CallbackError, got {r:?}"),
+    }
 
     Ok(())
 }