Panic on spurious Err(Throw) instead of silently coercing to undefined

crates/neon-sys/native/src/neon.cc

@@ -553,10 +553,10 @@ extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, voi
 
   if (!try_catch.HasCaught()) {
     // It's possible, if unlikely, that a Neon user might return `Err(Throw)` even
-    // though the VM is not actually in a throwing state. In this case we will
-    // simply produce the JS undefined value.
+    // though the VM is not actually in a throwing state. In this case we return
+    // `CONTROL_UNEXPECTED_ERR` to signal that Rust should panic.
     if (ctrl == CONTROL_THREW) {
-      *result = Nan::Undefined();
+      return CONTROL_UNEXPECTED_ERR;
     }
     return CONTROL_RETURNED;
   } else {

crates/neon-sys/native/src/neon.h

@@ -15,7 +15,8 @@ typedef struct {
 typedef enum : uint8_t {
   CONTROL_RETURNED = 0,
   CONTROL_THREW = 1,
-  CONTROL_PANICKED = 2
+  CONTROL_PANICKED = 2,
+  CONTROL_UNEXPECTED_ERR = 3
 } try_catch_control_t;
 
 extern "C" {
@@ -154,7 +155,7 @@ extern "C" {
   typedef try_catch_control_t (*Neon_TryCatchGlue)(void *rust_thunk, void *cx, v8::Local<v8::Value> *result, void **unwind_value);
 
   // The `result` out-parameter can be assumed to be initialized if and only if this function
-  // does not return `CONTROL_PANICKED`.
+  // returns `CONTROL_RETURNED` or `CONTROL_THREW`.
   // The `unwind_value` out-parameter can be assumed to be initialized if and only if this
   // function returns `CONTROL_PANICKED`.
   try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue, void *rust_thunk, void *cx, v8::Local<v8::Value> *result, void **unwind_value);

crates/neon-sys/src/lib.rs

@@ -82,7 +82,8 @@ impl Default for CCallback {
 pub enum TryCatchControl {
     Returned = 0,
     Threw = 1,
-    Panicked = 2
+    Panicked = 2,
+    UnexpectedErr = 3
 }
 
 #[derive(Clone, Copy)]

src/context/internal.rs

@@ -164,6 +164,9 @@ pub trait ContextInternal<'a>: Sized {
                 let local = unsafe { local.assume_init() };
                 Err(JsValue::new_internal(local))
             }
+            TryCatchControl::UnexpectedErr => {
+                panic!("try_catch: unexpected Err(Throw) when VM is not in a throwing state");
+            }
         }
     }
 
@@ -179,7 +182,7 @@ pub trait ContextInternal<'a>: Sized {
             } else if let Ok(result) = result {
                 Ok(result)
             } else {
-                Ok(JsUndefined::new_internal(self.env()).upcast())
+                panic!("try_catch: unexpected Err(Throw) when VM is not in a throwing state");
             }
         }
     }

test/dynamic/lib/functions.js

@@ -56,6 +56,10 @@ describe('JsFunction', function() {
     }, Error, /^internal error in Neon module: oh no$/);
   });
 
+  it('panics on unexpected Err(Throw) with cx.try_catch', function() {
+    assert.throw(addon.unexpected_throw_and_catch, Error, /^internal error in Neon module: try_catch: unexpected Err\(Throw\) when VM is not in a throwing state$/);
+  })
+
   it('computes the right number of arguments', function() {
     assert.equal(addon.num_arguments(), 0);
     assert.equal(addon.num_arguments('a'), 1);

test/dynamic/native/src/js/functions.rs

@@ -1,5 +1,6 @@
 use neon::prelude::*;
 use neon::object::This;
+use neon::result::Throw;
 
 fn add1(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let x = cx.argument::<JsNumber>(0)?.value();
@@ -121,3 +122,8 @@ pub fn panic_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
     Ok(cx.try_catch(|_| { panic!("oh no") })
          .unwrap_or_else(|err| err))
 }
+
+pub fn unexpected_throw_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
+    Ok(cx.try_catch(|_| { Err(Throw) })
+         .unwrap_or_else(|err| err))
+}

test/dynamic/native/src/lib.rs

@@ -73,6 +73,7 @@ register_module!(mut cx, {
     cx.export_function("throw_and_catch", throw_and_catch)?;
     cx.export_function("call_and_catch", call_and_catch)?;
     cx.export_function("panic_and_catch", panic_and_catch)?;
+    cx.export_function("unexpected_throw_and_catch", unexpected_throw_and_catch)?;
 
     cx.export_class::<JsEmitter>("Emitter")?;
     cx.export_class::<JsTestEmitter>("TestEmitter")?;