async_wrap: don't abort on callback exception

Rather than abort if the init/pre/post/final/destroy callbacks throw,
force the exception to propagate and not be made catchable. This way
the application is still not allowed to proceed but also allowed the
location of the failure to print before exiting. Though the stack itself
may not be of much use since all callbacks except init are called from
the bottom of the call stack.

    /tmp/async-test.js:14
      throw new Error('pre');
      ^
    Error: pre
        at InternalFieldObject.pre (/tmp/async-test.js:14:9)

PR-URL: https://github.com/nodejs/node/pull/5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

src/async-wrap-inl.h

@@ -51,11 +51,15 @@ inline AsyncWrap::AsyncWrap(Environment* env,
     argv[3] = parent->object();
   }
 
+  v8::TryCatch try_catch(env->isolate());
+
   v8::MaybeLocal<v8::Value> ret =
       init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv);
 
-  if (ret.IsEmpty())
+  if (ret.IsEmpty()) {
-    FatalError("node::AsyncWrap::AsyncWrap", "init hook threw");
+    ClearFatalExceptionHandlers(env);
+    FatalException(env->isolate(), try_catch);
+  }
 
   bits_ |= 1;  // ran_init_callback() is true now.
 }
@@ -69,10 +73,13 @@ inline AsyncWrap::~AsyncWrap() {
   if (!fn.IsEmpty()) {
     v8::HandleScope scope(env()->isolate());
     v8::Local<v8::Value> uid = v8::Integer::New(env()->isolate(), get_uid());
+    v8::TryCatch try_catch(env()->isolate());
     v8::MaybeLocal<v8::Value> ret =
         fn->Call(env()->context(), v8::Null(env()->isolate()), 1, &uid);
-    if (ret.IsEmpty())
+    if (ret.IsEmpty()) {
-      FatalError("node::AsyncWrap::~AsyncWrap", "destroy hook threw");
+      ClearFatalExceptionHandlers(env());
+      FatalException(env()->isolate(), try_catch);
+    }
   }
 }
 

src/async-wrap.cc

@@ -18,6 +18,7 @@ using v8::HeapProfiler;
 using v8::Integer;
 using v8::Isolate;
 using v8::Local;
+using v8::MaybeLocal;
 using v8::Object;
 using v8::RetainedObjectInfo;
 using v8::TryCatch;
@@ -225,8 +226,13 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   }
 
   if (ran_init_callback() && !pre_fn.IsEmpty()) {
-    if (pre_fn->Call(context, 1, &uid).IsEmpty())
+    TryCatch try_catch(env()->isolate());
-      FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
+    MaybeLocal<Value> ar = pre_fn->Call(env()->context(), context, 1, &uid);
+    if (ar.IsEmpty()) {
+      ClearFatalExceptionHandlers(env());
+      FatalException(env()->isolate(), try_catch);
+      return Local<Value>();
+    }
   }
 
   Local<Value> ret = cb->Call(context, argc, argv);
@@ -234,8 +240,14 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   if (ran_init_callback() && !post_fn.IsEmpty()) {
     Local<Value> did_throw = Boolean::New(env()->isolate(), ret.IsEmpty());
     Local<Value> vals[] = { uid, did_throw };
-    if (post_fn->Call(context, ARRAY_SIZE(vals), vals).IsEmpty())
+    TryCatch try_catch(env()->isolate());
-      FatalError("node::AsyncWrap::MakeCallback", "post hook threw");
+    MaybeLocal<Value> ar =
+        post_fn->Call(env()->context(), context, ARRAY_SIZE(vals), vals);
+    if (ar.IsEmpty()) {
+      ClearFatalExceptionHandlers(env());
+      FatalException(env()->isolate(), try_catch);
+      return Local<Value>();
+    }
   }
 
   if (ret.IsEmpty()) {

src/node.cc

@@ -1193,8 +1193,13 @@ Local<Value> MakeCallback(Environment* env,
   }
 
   if (ran_init_callback && !pre_fn.IsEmpty()) {
-    if (pre_fn->Call(object, 0, nullptr).IsEmpty())
+    TryCatch try_catch(env->isolate());
-      FatalError("node::MakeCallback", "pre hook threw");
+    MaybeLocal<Value> ar = pre_fn->Call(env->context(), object, 0, nullptr);
+    if (ar.IsEmpty()) {
+      ClearFatalExceptionHandlers(env);
+      FatalException(env->isolate(), try_catch);
+      return Local<Value>();
+    }
   }
 
   Local<Value> ret = callback->Call(recv, argc, argv);
@@ -1205,8 +1210,14 @@ Local<Value> MakeCallback(Environment* env,
     // This needs to be fixed.
     Local<Value> vals[] =
         { Undefined(env->isolate()).As<Value>(), did_throw };
-    if (post_fn->Call(object, ARRAY_SIZE(vals), vals).IsEmpty())
+    TryCatch try_catch(env->isolate());
-      FatalError("node::MakeCallback", "post hook threw");
+    MaybeLocal<Value> ar =
+        post_fn->Call(env->context(), object, ARRAY_SIZE(vals), vals);
+    if (ar.IsEmpty()) {
+      ClearFatalExceptionHandlers(env);
+      FatalException(env->isolate(), try_catch);
+      return Local<Value>();
+    }
   }
 
   if (ret.IsEmpty()) {
@@ -2404,6 +2415,25 @@ void OnMessage(Local<Message> message, Local<Value> error) {
 }
 
 
+void ClearFatalExceptionHandlers(Environment* env) {
+  Local<Object> process = env->process_object();
+  Local<Value> events =
+      process->Get(env->context(), env->events_string()).ToLocalChecked();
+
+  if (events->IsObject()) {
+    events.As<Object>()->Set(
+        env->context(),
+        OneByteString(env->isolate(), "uncaughtException"),
+        Undefined(env->isolate())).FromJust();
+  }
+
+  process->Set(
+      env->context(),
+      env->domain_string(),
+      Undefined(env->isolate())).FromJust();
+}
+
+
 static void Binding(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 

src/node_internals.h

@@ -236,6 +236,11 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
   Environment* env_;
 };
 
+// Clear any domain and/or uncaughtException handlers to force the error's
+// propagation and shutdown the process. Use this to force the process to exit
+// by clearing all callbacks that could handle the error.
+void ClearFatalExceptionHandlers(Environment* env);
+
 enum NodeInstanceType { MAIN, WORKER };
 
 class NodeInstanceData {

test/parallel/test-async-wrap-throw-from-callback.js

@@ -0,0 +1,68 @@
+'use strict';
+
+require('../common');
+const async_wrap = process.binding('async_wrap');
+const assert = require('assert');
+const crypto = require('crypto');
+const domain = require('domain');
+const spawn = require('child_process').spawn;
+const callbacks = [ 'init', 'pre', 'post', 'destroy' ];
+const toCall = process.argv[2];
+var msgCalled = 0;
+var msgReceived = 0;
+
+function init() {
+  if (toCall === 'init')
+    throw new Error('init');
+}
+function pre() {
+  if (toCall === 'pre')
+    throw new Error('pre');
+}
+function post() {
+  if (toCall === 'post')
+    throw new Error('post');
+}
+function destroy() {
+  if (toCall === 'destroy')
+    throw new Error('destroy');
+}
+
+if (typeof process.argv[2] === 'string') {
+  async_wrap.setupHooks({ init, pre, post, destroy });
+  async_wrap.enable();
+
+  process.on('uncaughtException', () => assert.ok(0, 'UNREACHABLE'));
+
+  const d = domain.create();
+  d.on('error', () => assert.ok(0, 'UNREACHABLE'));
+  d.run(() => {
+    // Using randomBytes because timers are not yet supported.
+    crypto.randomBytes(0, () => { });
+  });
+
+} else {
+
+  process.on('exit', (code) => {
+    assert.equal(msgCalled, callbacks.length);
+    assert.equal(msgCalled, msgReceived);
+  });
+
+  callbacks.forEach((item) => {
+    msgCalled++;
+
+    const child = spawn(process.execPath, [__filename, item]);
+    var errstring = '';
+
+    child.stderr.on('data', (data) => {
+      errstring += data.toString();
+    });
+
+    child.on('close', (code) => {
+      if (errstring.includes('Error: ' + item))
+        msgReceived++;
+
+      assert.equal(code, 1, `${item} closed with code ${code}`);
+    });
+  });
+}