Merge pull request #1135 from realm/yg/node-fatal-exceptions

Rethrow callback errors as fatal Node.js errors

CHANGELOG.md

@@ -1,3 +1,16 @@
+vNext Release notes (TBD)
+=============================================================
+### Breaking changes
+* None
+
+### Enhancements
+* None
+
+### Bug fixes
+* Fix crash on Node.js when a listener callback throws an error.
+  The error will now be forwarded to Node's fatal error handling facilities. This means better error reporting,
+  the ability to debug such errors in a Node.js debugger, and proper invocation of the `uncaughtError` event on the `process` object.
+
 1.9.0 Release notes (2017-7-10)
 =============================================================
 ### Breaking changes

lib/extensions.js

@@ -49,9 +49,13 @@ module.exports = function(realmConstructor) {
                         reject(error);
                     }
 
-                    let syncedRealm = new realmConstructor(config);
+                    try {
-                    //FIXME: RN hangs here. Remove when node's makeCallback alternative is implemented
+                        let syncedRealm = new this(config);
-                    setTimeout(() => { resolve(syncedRealm); }, 1);                     
+                        //FIXME: RN hangs here. Remove when node's makeCallback alternative is implemented
+                        setTimeout(() => { resolve(syncedRealm); }, 1);
+                    } catch (e) {
+                        reject(e);
+                    }
                 });
             });
         },
@@ -62,9 +66,13 @@ module.exports = function(realmConstructor) {
                         callback(error);
                     }
 
-                    let syncedRealm = new realmConstructor(config);
+                    try {
-                    //FIXME: RN hangs here. Remove when node's makeCallback alternative is implemented
+                        let syncedRealm = new this(config);
-                    setTimeout(() => { callback(null, syncedRealm); }, 1);
+                        //FIXME: RN hangs here. Remove when node's makeCallback alternative is implemented
+                        setTimeout(() => { callback(null, syncedRealm); }, 1);
+                    } catch (e) {
+                        callback(e);
+                    }
                 });
         },
     }));

src/node/node_function.hpp

@@ -38,13 +38,21 @@ inline v8::Local<v8::Value> node::Function::call(v8::Isolate* isolate, const v8:
 
 template<>
 inline v8::Local<v8::Value> node::Function::callback(v8::Isolate* isolate, const v8::Local<v8::Function> &function, const v8::Local<v8::Object> &this_object, size_t argc, const v8::Local<v8::Value> arguments[]) {
-    Nan::TryCatch trycatch;
+    if (!isolate->GetCallingContext().IsEmpty()) {
+        // if there are any JavaScript frames on the stack below this one we don't need to
+        // go through the trouble of calling MakeCallback. MakeCallback is only for when a
+        // thread with no JavaScript frames on its stack needs to call into JavaScript, like in
+        // an uv_async callback.
+        return call(isolate, function, this_object, argc, arguments);
+    }
+
+    v8::TryCatch trycatch(isolate);
     
     auto recv = this_object.IsEmpty() ? isolate->GetCurrentContext()->Global() : this_object;
-    auto result = Nan::MakeCallback(recv, function, (int)argc, const_cast<v8::Local<v8::Value>*>(arguments));
+    auto result = ::node::MakeCallback(isolate, recv, function, (int)argc, const_cast<v8::Local<v8::Value>*>(arguments));
 
     if (trycatch.HasCaught()) {
-        throw node::Exception(isolate, trycatch.Exception());
+        ::node::FatalException(isolate, trycatch);
     }
     return result;
 }