Big API incompatible error change, remove dependency on error_chain

The current situation with error_chain is less than ideal, and there are lots of
conflicting interests that are impossible to meet at once.  Here is an
unorganized brain dump of the current situation, stay awhile and listen!

This change was triggered ultimately by the desire to make LuaError implement
Clone, and this is currently impossible with error_chain.  LuaError must
implement Clone to be a proper lua citizen that can live as userdata within a
lua runtime, because there is no way to limit what the lua runtime can do with a
received error.  Currently, this is solved by there being a rule that the error
will "expire" if the error is passed back into rust, and this is very
sub-optimal.  In fact, one could easily imagine a scenario where lua is for
example memoizing some function, and if the function has ever errored in the
past the function should continue returning the same error, and this situation
immediately fails with this restriciton in place.

Additionally, there are other more minor problems with error_chain which make
the API less good than it could be, or limit how we can use error_chain.  This
change has already solved a small bug in a Chucklefish project, where the
conversion from an external error type (Borrow[Mut]Error) was allowed but not
intended for user code, and was accidentally used.  Additionally, pattern
matching on error_chain errors, which should be common when dealing with Lua, is
less convenient than a hand rolled error type.

So, if we decide not to use error_chain, we now have a new set of problems if we
decide interoperability with error_chain is important.  The first problem we run
into is that there are two natural bounds for wrapped errors that we would
pick, (Error + Send + Sync), or just Error, and neither of them will
interoperate well with error_chain.  (Error + Send + Sync) means we can't wrap
error chain errors into LuaError::ExternalError (they're missing the Sync
bound), and having the bounds be just Error means the opposite, that we can't
hold a LuaError inside an error_chain error.

We could just decide that interoperability with error_chain is the most
important qualification, and pick (Error + Send), but this causes a DIFFERENT
set of problems.  The rust ecosystem has the two primary error bounds as Error
or (Error + Send + Sync), and there are Into impls from &str / String to
Box<Error + Send + Sync> for example, but NOT (Error + Send).  This means that
we are forced to manually recreate the conversions from &str / String to
LuaError rather than relying on a single Into<Box<Error + Send + Sync>> bound,
but this means that string conversions have a different set of methods than
other error types for external error conversion.  I have not been able to figure
out an API that I am happy with that uses the (Error + Send) bound.  Box<Error>
is obnoxious because not having errors implement Send causes needless problems
in a multithreaded context, so that leaves (Error + Send + Sync).  This is
actually a completely reasonable bound for external errors, and has the nice
String Into impls that we would want, the ONLY problem is that it is a pain to
interoperate with the current version of error_chain.

It would be nice to be able to specify the traits that an error generated by the
error_chain macro would implement, and this is apparently in progress in the
error_chain library.  This would solve both the problem with not being able to
implement Clone and the problems with (Error + Send) bounds.  I am not convinced
that this library should go back to using error_chain when that functionality is
in stable error_chain though, because of the other minor usability problems with
using error_chain.

In that theoretical situation, the downside of NOT using error_chain is simply
that there would not be automatic stacktraces of LuaError.  This is not a huge
problem, because stack traces of lua errors are not extremely useful, and for
external errors it is not too hard to create a different version of the
LuaExternalResult / LuaExternalError traits and do conversion from an
error_chain type into a type that will print the stacktrace on display, or
use downcasting in the error causes.

So in summary, this library is no longer using error_chain, and probably will
not use it again in the future.  Currently this means that to interoperate with
error_chain, you should use error_chain 0.8.1, which derives Sync on errors, or
wait for a version that supports user defined trait derives.  In the future
when error_chain supports user defined trait derives, users may have to take an
extra step to make wrapped external errors print the stacktrace that they
capture.

This change works, but is not entirely complete.  There is no error
documentation yet, and the change brought to a head an ugly module organization
problem.  There will be more commits for documentation and reorganization, then
a new stable version of rlua.

Cargo.toml

@@ -14,4 +14,3 @@ gcc = "0.3"
 
 [dependencies]
 hlist-macro = "0.1"
-error-chain = "0.10"

examples/repl.rs

@@ -32,7 +32,7 @@ fn main() {
                     );
                     break;
                 }
-                Err(LuaError(LuaErrorKind::IncompleteStatement(_), _)) => {
+                Err(LuaError::IncompleteStatement(_)) => {
                     // continue reading input and append it to `line`
                     write!(stdout, ">> ").unwrap();
                     stdout.flush().unwrap();

src/conversion.rs

@@ -38,7 +38,9 @@ impl<'lua> FromLua<'lua> for LuaTable<'lua> {
     fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult<LuaTable<'lua>> {
         match value {
             LuaValue::Table(table) => Ok(table),
-            _ => Err("cannot convert lua value to table".into()),
+            _ => Err(LuaError::ConversionError(
+                "cannot convert lua value to table".to_owned(),
+            )),
         }
     }
 }
@@ -53,7 +55,9 @@ impl<'lua> FromLua<'lua> for LuaFunction<'lua> {
     fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult<LuaFunction<'lua>> {
         match value {
             LuaValue::Function(table) => Ok(table),
-            _ => Err("cannot convert lua value to function".into()),
+            _ => Err(LuaError::ConversionError(
+                "cannot convert lua value to function".to_owned(),
+            )),
         }
     }
 }
@@ -68,7 +72,9 @@ impl<'lua> FromLua<'lua> for LuaUserData<'lua> {
     fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult<LuaUserData<'lua>> {
         match value {
             LuaValue::UserData(ud) => Ok(ud),
-            _ => Err("cannot convert lua value to userdata".into()),
+            _ => Err(LuaError::ConversionError(
+                "cannot convert lua value to userdata".to_owned(),
+            )),
         }
     }
 }
@@ -83,7 +89,9 @@ impl<'lua> FromLua<'lua> for LuaThread<'lua> {
     fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult<LuaThread<'lua>> {
         match value {
             LuaValue::Thread(t) => Ok(t),
-            _ => Err("cannot convert lua value to thread".into()),
+            _ => Err(LuaError::ConversionError(
+                "cannot convert lua value to thread".to_owned(),
+            )),
         }
     }
 }
@@ -98,7 +106,9 @@ impl<'lua, T: LuaUserDataType + Copy> FromLua<'lua> for T {
     fn from_lua(value: LuaValue<'lua>, _: &'lua Lua) -> LuaResult<T> {
         match value {
             LuaValue::UserData(ud) => Ok(*ud.borrow::<T>()?),
-            _ => Err("cannot convert lua value to userdata".into()),
+            _ => Err(LuaError::ConversionError(
+                "cannot convert lua value to userdata".to_owned(),
+            )),
         }
     }
 }
@@ -129,7 +139,9 @@ impl<'lua> FromLua<'lua> for LightUserData {
     fn from_lua(v: LuaValue, _: &'lua Lua) -> LuaResult<Self> {
         match v {
             LuaValue::LightUserData(ud) => Ok(ud),
-            _ => Err("cannot convert lua value to lightuserdata".into()),
+            _ => Err(LuaError::ConversionError(
+                "cannot convert lua value to lightuserdata".to_owned(),
+            )),
         }
     }
 }
@@ -209,7 +221,9 @@ impl<'lua, T: FromLua<'lua>> FromLua<'lua> for Vec<T> {
         if let LuaValue::Table(table) = value {
             table.sequence_values().collect()
         } else {
-            Err("cannot convert lua value to table for Vec".into())
+            Err(LuaError::ConversionError(
+                "cannot convert lua value to table for Vec".to_owned(),
+            ))
         }
     }
 }
@@ -225,7 +239,9 @@ impl<'lua, K: Eq + Hash + FromLua<'lua>, V: FromLua<'lua>> FromLua<'lua> for Has
         if let LuaValue::Table(table) = value {
             table.pairs().collect()
         } else {
-            Err("cannot convert lua value to table for HashMap".into())
+            Err(LuaError::ConversionError(
+                "cannot convert lua value to table for HashMap".to_owned(),
+            ))
         }
     }
 }
@@ -241,7 +257,9 @@ impl<'lua, K: Ord + FromLua<'lua>, V: FromLua<'lua>> FromLua<'lua> for BTreeMap<
         if let LuaValue::Table(table) = value {
             table.pairs().collect()
         } else {
-            Err("cannot convert lua value to table for BTreeMap".into())
+            Err(LuaError::ConversionError(
+                "cannot convert lua value to table for BTreeMap".to_owned(),
+            ))
         }
     }
 }

src/error.rs

@@ -1,68 +1,122 @@
 use std::fmt;
+use std::sync::Arc;
 use std::result::Result;
 use std::error::Error;
 use std::ffi::NulError;
-use std::cell::{BorrowError, BorrowMutError};
 use std::str::Utf8Error;
 
-#[derive(Debug)]
-pub struct LuaExternalError(pub Box<Error + Send>);
+#[derive(Debug, Clone)]
+pub enum LuaError {
+    ScriptError(String),
+    CallbackError(String, Arc<LuaError>),
+    IncompleteStatement(String),
+    CoroutineInactive,
+    StackOverflow,
+    UserDataBorrowError,
+    UserDataBorrowMutError,
+    Utf8Error(Utf8Error),
+    NulError(NulError),
+    ConversionError(String),
+    ExternalError(Arc<Error + Send + Sync>),
+}
 
-impl fmt::Display for LuaExternalError {
+pub type LuaResult<T> = Result<T, LuaError>;
+
+impl fmt::Display for LuaError {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        self.0.fmt(fmt)
+        match self {
+            &LuaError::ScriptError(ref msg) => write!(fmt, "Error executing lua script: {}", msg),
+            &LuaError::CallbackError(ref msg, _) => {
+                write!(fmt, "Error during lua callback: {}", msg)
+            }
+            &LuaError::ExternalError(ref err) => err.fmt(fmt),
+            &LuaError::IncompleteStatement(ref msg) => {
+                write!(fmt, "Incomplete lua statement: {}", msg)
+            }
+            &LuaError::CoroutineInactive => write!(fmt, "Cannot resume inactive coroutine"),
+            &LuaError::ConversionError(ref msg) => {
+                write!(fmt, "Error converting lua type: {}", msg)
+            }
+            &LuaError::StackOverflow => write!(fmt, "Lua stack overflow"),
+            &LuaError::UserDataBorrowError => write!(fmt, "Userdata already mutably borrowed"),
+            &LuaError::UserDataBorrowMutError => write!(fmt, "Userdata already borrowed"),
+            &LuaError::Utf8Error(ref err) => write!(fmt, "Lua string utf8 error: {}", err),
+            &LuaError::NulError(ref err) => {
+                write!(fmt, "String passed to lua contains null: {}", err)
+            }
+        }
     }
 }
 
-impl Error for LuaExternalError {
+impl Error for LuaError {
     fn description(&self) -> &str {
-        self.0.description()
+        match self {
+            &LuaError::ScriptError(_) => "lua script error",
+            &LuaError::CallbackError(_, _) => "lua callback error",
+            &LuaError::ExternalError(ref err) => err.description(),
+            &LuaError::IncompleteStatement(_) => "lua incomplete statement",
+            &LuaError::CoroutineInactive => "lua coroutine inactive",
+            &LuaError::ConversionError(_) => "lua conversion error",
+            &LuaError::StackOverflow => "lua stack overflow",
+            &LuaError::UserDataBorrowError => "lua userdata already mutably borrowed",
+            &LuaError::UserDataBorrowMutError => "lua userdata already borrowed",
+            &LuaError::Utf8Error(_) => "lua string utf8 conversion error",
+            &LuaError::NulError(_) => "string null error",
+        }
     }
 
     fn cause(&self) -> Option<&Error> {
-        self.0.cause()
+        match self {
+            &LuaError::CallbackError(_, ref cause) => Some(cause.as_ref()),
+            &LuaError::ExternalError(ref err) => err.cause(),
+            _ => None,
+        }
     }
 }
 
-error_chain! {
-    types {
-        LuaError, LuaErrorKind, LuaResultExt, LuaResult;
-    }
-
-    errors {
-        ScriptError(err: String) {
-            display("Error executing lua script {}", err)
-        }
-        CallbackError(err: String) {
-            display("Error during lua callback {}", err)
-        }
-        IncompleteStatement(err: String) {
-            display("Incomplete lua statement {}", err)
-        }
-        CoroutineInactive {
-            display("Cannot resume inactive coroutine")
-        }
-    }
-
-    foreign_links {
-        ExternalError(LuaExternalError);
-        Utf8Error(Utf8Error);
-        NulError(NulError);
-        BorrowError(BorrowError);
-        BorrowMutError(BorrowMutError);
+impl LuaError {
+    pub fn external<T: 'static + Error + Send + Sync>(err: T) -> LuaError {
+        LuaError::ExternalError(Arc::new(err))
+    }
+}
+
+pub trait LuaExternalError {
+    fn to_lua_err(self) -> LuaError;
+}
+
+impl<E> LuaExternalError for E
+where
+    E: Into<Box<Error + Send + Sync>>,
+{
+    fn to_lua_err(self) -> LuaError {
+        #[derive(Debug)]
+        struct WrapError(Box<Error + Send + Sync>);
+
+        impl fmt::Display for WrapError {
+            fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+                self.0.fmt(f)
+            }
+        }
+
+        impl Error for WrapError {
+            fn description(&self) -> &str {
+                self.0.description()
+            }
+        }
+
+        LuaError::external(WrapError(self.into()))
     }
 }
 
-/// Helper trait to convert external error types to a `LuaExternalError`
 pub trait LuaExternalResult<T> {
     fn to_lua_err(self) -> LuaResult<T>;
 }
 
 impl<T, E> LuaExternalResult<T> for Result<T, E>
 where
-    E: 'static + Error + Send,
+    E: LuaExternalError,
 {
     fn to_lua_err(self) -> LuaResult<T> {
-        self.map_err(|e| LuaExternalError(Box::new(e)).into())
+        self.map_err(|e| e.to_lua_err())
     }
 }

src/lib.rs

@@ -1,9 +1,6 @@
 #[cfg_attr(test, macro_use)]
 extern crate hlist_macro;
 
-#[macro_use]
-extern crate error_chain;
-
 pub mod ffi;
 mod util;
 mod error;

src/lua.rs

@@ -98,8 +98,9 @@ impl<'lua> DerefMut for LuaMultiValue<'lua> {
 
 /// Trait for types convertible to any number of Lua values.
 ///
-/// This is a generalization of `ToLua`, allowing any number of resulting Lua values instead of just
-/// one. Any type that implements `ToLua` will automatically implement this trait.
+/// This is a generalization of `ToLua`, allowing any number of resulting Lua
+/// values instead of just one. Any type that implements `ToLua` will
+/// automatically implement this trait.
 pub trait ToLuaMulti<'a> {
     /// Performs the conversion.
     fn to_lua_multi(self, lua: &'a Lua) -> LuaResult<LuaMultiValue<'a>>;
@@ -107,14 +108,17 @@ pub trait ToLuaMulti<'a> {
 
 /// Trait for types that can be created from an arbitrary number of Lua values.
 ///
-/// This is a generalization of `FromLua`, allowing an arbitrary number of Lua values to participate
-/// in the conversion. Any type that implements `FromLua` will automatically implement this trait.
+/// This is a generalization of `FromLua`, allowing an arbitrary number of Lua
+/// values to participate in the conversion. Any type that implements `FromLua`
+/// will automatically implement this trait.
 pub trait FromLuaMulti<'a>: Sized {
     /// Performs the conversion.
     ///
-    /// In case `values` contains more values than needed to perform the conversion, the excess
-    /// values should be ignored. This reflects the semantics of Lua when calling a function or
-    /// assigning values. Of course, if not enough values are given, an error should be returned.
+    /// In case `values` contains more values than needed to perform the
+    /// conversion, the excess values should be ignored. This reflects the
+    /// semantics of Lua when calling a function or assigning values. Similarly,
+    /// if not enough values are given, conversions should assume that any
+    /// missing values are nil.
     fn from_lua_multi(values: LuaMultiValue<'a>, lua: &'a Lua) -> LuaResult<Self>;
 }
 
@@ -201,7 +205,9 @@ impl<'lua> LuaString<'lua> {
                 check_stack(lua.state, 1)?;
                 lua.push_ref(lua.state, &self.0);
                 assert_eq!(ffi::lua_type(lua.state, -1), ffi::LUA_TSTRING);
-                let s = CStr::from_ptr(ffi::lua_tostring(lua.state, -1)).to_str()?;
+                let s = CStr::from_ptr(ffi::lua_tostring(lua.state, -1))
+                    .to_str()
+                    .map_err(|e| LuaError::Utf8Error(e))?;
                 ffi::lua_pop(lua.state, 1);
                 Ok(s)
             })
@@ -218,8 +224,8 @@ impl<'lua> LuaTable<'lua> {
     ///
     /// If the value is `nil`, this will effectively remove the pair.
     ///
-    /// This might invoke the `__newindex` metamethod. Use the `raw_set` method if that is not
-    /// desired.
+    /// This might invoke the `__newindex` metamethod. Use the `raw_set` method
+    /// if that is not desired.
     pub fn set<K: ToLua<'lua>, V: ToLua<'lua>>(&self, key: K, value: V) -> LuaResult<()> {
         let lua = self.0.lua;
         let key = key.to_lua(lua)?;
@@ -240,7 +246,8 @@ impl<'lua> LuaTable<'lua> {
     ///
     /// If no value is associated to `key`, returns the `nil` value.
     ///
-    /// This might invoke the `__index` metamethod. Use the `raw_get` method if that is not desired.
+    /// This might invoke the `__index` metamethod. Use the `raw_get` method if
+    /// that is not desired.
     pub fn get<K: ToLua<'lua>, V: FromLua<'lua>>(&self, key: K) -> LuaResult<V> {
         let lua = self.0.lua;
         let key = key.to_lua(lua)?;
@@ -638,7 +645,7 @@ impl<'lua> LuaThread<'lua> {
     ///
     /// // The coroutine has now returned, so `resume` will fail
     /// match thread.resume::<_, u32>(()) {
-    ///     Err(LuaError(LuaErrorKind::CoroutineInactive, _)) => {},
+    ///     Err(LuaError::CoroutineInactive) => {},
     ///     unexpected => panic!("unexpected result {:?}", unexpected),
     /// }
     /// # }
@@ -658,7 +665,7 @@ impl<'lua> LuaThread<'lua> {
 
                 let status = ffi::lua_status(thread_state);
                 if status != ffi::LUA_YIELD && ffi::lua_gettop(thread_state) == 0 {
-                    return Err(LuaErrorKind::CoroutineInactive.into());
+                    return Err(LuaError::CoroutineInactive);
                 }
 
                 ffi::lua_pop(lua.state, 1);
@@ -848,7 +855,10 @@ impl<T: LuaUserDataType> LuaUserDataMethods<T> {
             let userdata = userdata.borrow::<T>()?;
             method(lua, &userdata, args)
         } else {
-            Err("No userdata supplied as first argument to method".into())
+            Err(LuaError::ConversionError(
+                "No userdata supplied as first argument to method"
+                    .to_owned(),
+            ))
         })
 
     }
@@ -862,7 +872,10 @@ impl<T: LuaUserDataType> LuaUserDataMethods<T> {
             let mut userdata = userdata.borrow_mut::<T>()?;
             method(lua, &mut userdata, args)
         } else {
-            Err("No userdata supplied as first argument to method".into())
+            Err(LuaError::ConversionError(
+                "No userdata supplied as first argument to method"
+                    .to_owned(),
+            ))
         })
 
     }
@@ -888,12 +901,20 @@ impl<'lua> LuaUserData<'lua> {
 
     /// Borrow this userdata out of the internal RefCell that is held in lua.
     pub fn borrow<T: LuaUserDataType>(&self) -> LuaResult<Ref<T>> {
-        self.inspect(|cell| Ok(cell.try_borrow()?))
+        self.inspect(|cell| {
+            Ok(
+                cell.try_borrow().map_err(|_| LuaError::UserDataBorrowError)?,
+            )
+        })
     }
 
     /// Borrow mutably this userdata out of the internal RefCell that is held in lua.
     pub fn borrow_mut<T: LuaUserDataType>(&self) -> LuaResult<RefMut<T>> {
-        self.inspect(|cell| Ok(cell.try_borrow_mut()?))
+        self.inspect(|cell| {
+            Ok(cell.try_borrow_mut().map_err(
+                |_| LuaError::UserDataBorrowMutError,
+            )?)
+        })
     }
 
     fn inspect<'a, T, R, F>(&'a self, func: F) -> LuaResult<R>
@@ -909,11 +930,13 @@ impl<'lua> LuaUserData<'lua> {
                 lua.push_ref(lua.state, &self.0);
                 let userdata = ffi::lua_touserdata(lua.state, -1);
                 if userdata.is_null() {
-                    return Err("value not userdata".into());
+                    return Err(LuaError::ConversionError("value not userdata".to_owned()));
                 }
 
                 if ffi::lua_getmetatable(lua.state, -1) == 0 {
-                    return Err("value has no metatable".into());
+                    return Err(LuaError::ConversionError(
+                        "value has no metatable".to_owned(),
+                    ));
                 }
 
                 ffi::lua_rawgeti(
@@ -922,7 +945,9 @@ impl<'lua> LuaUserData<'lua> {
                     lua.userdata_metatable::<T>()? as ffi::lua_Integer,
                 );
                 if ffi::lua_rawequal(lua.state, -1, -2) == 0 {
-                    return Err("wrong metatable type for lua userdata".into());
+                    return Err(LuaError::ConversionError(
+                        "wrong metatable type for lua userdata".to_owned(),
+                    ));
                 }
 
                 let res = func(&*(userdata as *const RefCell<T>));
@@ -1039,7 +1064,9 @@ impl Lua {
                 handle_error(
                     self.state,
                     if let Some(name) = name {
-                        let name = CString::new(name.to_owned())?;
+                        let name = CString::new(name.to_owned()).map_err(
+                            |e| LuaError::NulError(e),
+                        )?;
                         ffi::luaL_loadbuffer(
                             self.state,
                             source.as_ptr() as *const c_char,
@@ -1233,7 +1260,9 @@ impl Lua {
                     check_stack(self.state, 1)?;
                     self.push_value(self.state, v);
                     if ffi::lua_tostring(self.state, -1).is_null() {
-                        Err("cannot convert lua value to string".into())
+                        Err(LuaError::ConversionError(
+                            "cannot convert lua value to string".to_owned(),
+                        ))
                     } else {
                         Ok(LuaString(self.pop_ref(self.state)))
                     }
@@ -1256,7 +1285,9 @@ impl Lua {
                     let mut isint = 0;
                     let i = ffi::lua_tointegerx(self.state, -1, &mut isint);
                     if isint == 0 {
-                        Err("cannot convert lua value to integer".into())
+                        Err(LuaError::ConversionError(
+                            "cannot convert lua value to integer".to_owned(),
+                        ))
                     } else {
                         ffi::lua_pop(self.state, 1);
                         Ok(i)
@@ -1280,7 +1311,9 @@ impl Lua {
                     let mut isnum = 0;
                     let n = ffi::lua_tonumberx(self.state, -1, &mut isnum);
                     if isnum == 0 {
-                        Err("cannot convert lua value to number".into())
+                        Err(LuaError::ConversionError(
+                            "cannot convert lua value to number".to_owned(),
+                        ))
                     } else {
                         ffi::lua_pop(self.state, 1);
                         Ok(n)

src/tests.rs

@@ -68,7 +68,7 @@ fn test_eval() {
     assert_eq!(lua.eval::<bool>("false == false").unwrap(), true);
     assert_eq!(lua.eval::<i32>("return 1 + 2").unwrap(), 3);
     match lua.eval::<()>("if true then") {
-        Err(LuaError(LuaErrorKind::IncompleteStatement(_), _)) => {}
+        Err(LuaError::IncompleteStatement(_)) => {}
         r => panic!("expected IncompleteStatement, got {:?}", r),
     }
 }
@@ -313,7 +313,7 @@ fn test_metamethods() {
                 if index.to_str()? == "inner" {
                     lua.pack(data.0)
                 } else {
-                    Err("no such custom index".into())
+                    Err("no such custom index".to_lua_err())
                 }
             });
         }
@@ -481,9 +481,8 @@ fn test_error() {
         None,
     ).unwrap();
 
-    let rust_error_function = lua.create_function(
-        |_, _| Err(LuaExternalError(Box::new(TestError)).into()),
-    ).unwrap();
+    let rust_error_function = lua.create_function(|_, _| Err(TestError.to_lua_err()))
+        .unwrap();
     globals
         .set("rust_error_function", rust_error_function)
         .unwrap();
@@ -498,12 +497,12 @@ fn test_error() {
 
     assert!(no_error.call::<_, ()>(()).is_ok());
     match lua_error.call::<_, ()>(()) {
-        Err(LuaError(LuaErrorKind::ScriptError(_), _)) => {}
+        Err(LuaError::ScriptError(_)) => {}
         Err(_) => panic!("error is not ScriptError kind"),
         _ => panic!("error not returned"),
     }
     match rust_error.call::<_, ()>(()) {
-        Err(LuaError(LuaErrorKind::CallbackError(_), _)) => {}
+        Err(LuaError::CallbackError(_, _)) => {}
         Err(_) => panic!("error is not CallbackError kind"),
         _ => panic!("error not returned"),
     }
@@ -637,7 +636,7 @@ fn test_thread() {
     assert_eq!(thread.resume::<_, u32>(43).unwrap(), 987);
 
     match thread.resume::<_, u32>(()) {
-        Err(LuaError(LuaErrorKind::CoroutineInactive, _)) => {}
+        Err(LuaError::CoroutineInactive) => {}
         Err(_) => panic!("resuming dead coroutine error is not CoroutineInactive kind"),
         _ => panic!("resuming dead coroutine did not return error"),
     }
@@ -700,13 +699,12 @@ fn test_result_conversions() {
     let globals = lua.globals().unwrap();
 
     let err = lua.create_function(|lua, _| {
-        lua.pack(Result::Err::<String, LuaError>(
-            "only through failure can we succeed".into(),
+        lua.pack(Result::Err::<String, _>(
+            "only through failure can we succeed".to_lua_err(),
         ))
     }).unwrap();
-    let ok = lua.create_function(|lua, _| {
-        lua.pack(Result::Ok::<String, LuaError>("!".to_string()))
-    }).unwrap();
+    let ok = lua.create_function(|lua, _| lua.pack(Result::Ok::<_, LuaError>("!".to_owned())))
+        .unwrap();
 
     globals.set("err", err).unwrap();
     globals.set("ok", ok).unwrap();
@@ -716,7 +714,6 @@ fn test_result_conversions() {
             local r, e = err()
             assert(r == nil)
             assert(tostring(e) == "only through failure can we succeed")
-            assert(type(e:backtrace()) == "string")
 
             local r, e = ok()
             assert(r == "!")

src/util.rs

@@ -1,13 +1,13 @@
-use std::ptr;
 use std::mem;
+use std::ptr;
+use std::sync::Arc;
 use std::ffi::CStr;
 use std::any::Any;
 use std::os::raw::{c_char, c_int, c_void};
 use std::panic::{catch_unwind, resume_unwind, UnwindSafe};
-use error_chain::ChainedError;
 
 use ffi;
-use error::{LuaResult, LuaError, LuaErrorKind};
+use error::{LuaResult, LuaError};
 
 macro_rules! cstr {
   ($s:expr) => (
@@ -17,7 +17,7 @@ macro_rules! cstr {
 
 pub unsafe fn check_stack(state: *mut ffi::lua_State, amount: c_int) -> LuaResult<()> {
     if ffi::lua_checkstack(state, amount) == 0 {
-        Err("out of lua stack space".into())
+        Err(LuaError::StackOverflow)
     } else {
         Ok(())
     }
@@ -172,7 +172,7 @@ pub unsafe fn push_error(state: *mut ffi::lua_State, err: LuaError) {
 
 // Pushes a WrappedError::Panic to the top of the stack
 pub unsafe fn push_panic(state: *mut ffi::lua_State, panic: Box<Any + Send>) {
-    push_wrapped_error(state, WrappedError::Panic(panic));
+    push_wrapped_error(state, WrappedError::Panic(Some(panic)));
 }
 
 // Pops a WrappedError off of the top of the stack, if it is a WrappedError::Error, returns it, if
@@ -188,14 +188,16 @@ pub unsafe fn pop_error(state: *mut ffi::lua_State) -> LuaError {
 
     if is_wrapped_error(state, -1) {
         let userdata = ffi::lua_touserdata(state, -1);
-        let err = (*(userdata as *mut Option<WrappedError>))
-            .take()
-            .unwrap_or_else(|| WrappedError::Error("consumed error".into()));
-        ffi::lua_pop(state, 1);
-
-        match err {
-            WrappedError::Error(err) => err,
-            WrappedError::Panic(p) => {
+        match &mut *(userdata as *mut WrappedError) {
+            &mut WrappedError::Error(ref err) => {
+                let err = err.clone();
+                ffi::lua_pop(state, 1);
+                err
+            }
+            &mut WrappedError::Panic(ref mut p) => {
+                let p = p.take().unwrap_or_else(|| {
+                    Box::new("internal error: panic error used twice")
+                });
                 ffi::lua_settop(state, 0);
                 resume_unwind(p)
             }
@@ -207,20 +209,20 @@ pub unsafe fn pop_error(state: *mut ffi::lua_State) -> LuaError {
         // This seems terrible, but as far as I can tell, this is exactly what the stock lua
         // repl does.
         if error.ends_with("<eof>") {
-            LuaErrorKind::IncompleteStatement(error).into()
+            LuaError::IncompleteStatement(error).into()
         } else {
-            LuaErrorKind::ScriptError(error).into()
+            LuaError::ScriptError(error).into()
         }
 
     } else {
         ffi::lua_pop(state, 1);
-        LuaErrorKind::ScriptError("<unprintable error>".to_owned()).into()
+        LuaError::ScriptError("<unprintable error>".to_owned()).into()
     }
 }
 
-// ffi::lua_pcall with a message handler that gives a nice traceback.  If the caught error is
-// actually a LuaError, will simply pass the error along.  Does not call
-// checkstack, and uses 2 extra stack spaces.
+// ffi::lua_pcall with a message handler that gives a nice traceback.  If the
+// caught error is actually a LuaError, will simply pass the error along.  Does
+// not call checkstack, and uses 2 extra stack spaces.
 pub unsafe fn pcall_with_traceback(
     state: *mut ffi::lua_State,
     nargs: c_int,
@@ -235,10 +237,7 @@ pub unsafe fn pcall_with_traceback(
                     .to_str()
                     .unwrap()
                     .to_owned();
-                push_error(
-                    state,
-                    LuaError::with_chain(error, LuaErrorKind::CallbackError(traceback)),
-                );
+                push_error(state, LuaError::CallbackError(traceback, Arc::new(error)));
             }
         } else {
             let s = ffi::lua_tolstring(state, 1, ptr::null_mut());
@@ -274,10 +273,7 @@ pub unsafe fn resume_with_traceback(
                     .to_str()
                     .unwrap()
                     .to_owned();
-                push_error(
-                    from,
-                    LuaError::with_chain(error, LuaErrorKind::CallbackError(traceback)),
-                );
+                push_error(from, LuaError::CallbackError(traceback, Arc::new(error)));
             }
         } else {
             let s = ffi::lua_tolstring(state, 1, ptr::null_mut());
@@ -340,7 +336,7 @@ static ERROR_METATABLE_REGISTRY_KEY: u8 = 0;
 
 enum WrappedError {
     Error(LuaError),
-    Panic(Box<Any + Send>),
+    Panic(Option<Box<Any + Send>>),
 }
 
 // Pushes the given error or panic as a wrapped error onto the stack
@@ -351,53 +347,23 @@ unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: WrappedError) {
     unsafe extern "C" fn error_tostring(state: *mut ffi::lua_State) -> c_int {
         callback_error(state, || {
             if !is_wrapped_error(state, -1) {
-                return Err("not wrapped error in error method".into());
+                return Err(LuaError::ConversionError(
+                    "not WrappedError in error method".to_owned(),
+                ));
             }
 
             let userdata = ffi::lua_touserdata(state, -1);
-            match (*(userdata as *mut Option<WrappedError>)).as_ref() {
-                Some(&WrappedError::Error(ref error)) => {
+            match &*(userdata as *const WrappedError) {
+                &WrappedError::Error(ref error) => {
                     push_string(state, &error.to_string());
                     ffi::lua_remove(state, -2);
                 }
-                Some(&WrappedError::Panic(_)) => {
+                &WrappedError::Panic(_) => {
                     // This should be impossible, there should be no way for lua
                     // to catch a panic error.
                     push_string(state, "panic error");
                     ffi::lua_remove(state, -2);
                 }
-                None => {
-                    push_string(state, "consumed error");
-                    ffi::lua_remove(state, -2);
-                }
-            }
-
-            Ok(1)
-        })
-    }
-
-    unsafe extern "C" fn error_backtrace(state: *mut ffi::lua_State) -> c_int {
-        callback_error(state, || {
-            if !is_wrapped_error(state, -1) {
-                return Err("not wrapped error in error method".into());
-            }
-
-            let userdata = ffi::lua_touserdata(state, -1);
-            match (*(userdata as *mut Option<WrappedError>)).as_ref() {
-                Some(&WrappedError::Error(ref error)) => {
-                    push_string(state, &error.display().to_string());
-                    ffi::lua_remove(state, -2);
-                }
-                Some(&WrappedError::Panic(_)) => {
-                    // This should be impossible, there should be no way for lua
-                    // to catch a panic error.
-                    push_string(state, "panic error");
-                    ffi::lua_remove(state, -2);
-                }
-                None => {
-                    push_string(state, "consumed error");
-                    ffi::lua_remove(state, -2);
-                }
             }
 
             Ok(1)
@@ -406,10 +372,10 @@ unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: WrappedError) {
 
     ffi::luaL_checkstack(state, 2, ptr::null());
 
-    let err_userdata = ffi::lua_newuserdata(state, mem::size_of::<Option<WrappedError>>()) as
-        *mut Option<WrappedError>;
+    let err_userdata = ffi::lua_newuserdata(state, mem::size_of::<WrappedError>()) as
+        *mut WrappedError;
 
-    ptr::write(err_userdata, Some(err));
+    ptr::write(err_userdata, err);
 
     get_error_metatable(state);
     if ffi::lua_isnil(state, -1) != 0 {
@@ -425,20 +391,13 @@ unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: WrappedError) {
         ffi::lua_pushvalue(state, -2);
 
         push_string(state, "__gc");
-        ffi::lua_pushcfunction(state, destructor::<Option<WrappedError>>);
+        ffi::lua_pushcfunction(state, destructor::<WrappedError>);
         ffi::lua_settable(state, -3);
 
         push_string(state, "__tostring");
         ffi::lua_pushcfunction(state, error_tostring);
         ffi::lua_settable(state, -3);
 
-        push_string(state, "__index");
-        ffi::lua_newtable(state);
-        push_string(state, "backtrace");
-        ffi::lua_pushcfunction(state, error_backtrace);
-        ffi::lua_settable(state, -3);
-        ffi::lua_settable(state, -3);
-
         push_string(state, "__metatable");
         ffi::lua_pushboolean(state, 0);
         ffi::lua_settable(state, -3);
@@ -475,17 +434,10 @@ unsafe fn is_panic_error(state: *mut ffi::lua_State, index: c_int) -> bool {
     }
 
     ffi::lua_pop(state, 2);
-    let userdata_err = userdata as *mut Option<WrappedError>;
-    match (*userdata_err).take() {
-        Some(WrappedError::Error(err)) => {
-            *userdata_err = Some(WrappedError::Error(err));
-            false
-        }
-        Some(WrappedError::Panic(p)) => {
-            *userdata_err = Some(WrappedError::Panic(p));
-            true
-        }
-        None => false,
+
+    match &*(userdata as *const WrappedError) {
+        &WrappedError::Error(_) => false,
+        &WrappedError::Panic(_) => true,
     }
 }