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)

Ref: #7048
PR-URL: #5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

Author: trevnorris

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, arraysize(argv), argv);
 
-  if (ret.IsEmpty())
-    FatalError("node::AsyncWrap::AsyncWrap", "init hook threw");
+  if (ret.IsEmpty()) {
+    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())
-      FatalError("node::AsyncWrap::~AsyncWrap", "destroy hook threw");
+    if (ret.IsEmpty()) {
+      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,17 +226,28 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   }
 
   if (ran_init_callback() && !pre_fn.IsEmpty()) {
-    if (pre_fn->Call(context, 1, &uid).IsEmpty())
-      FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
+    TryCatch try_catch(env()->isolate());
+    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);
 
   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, arraysize(vals), vals).IsEmpty())
-      FatalError("node::AsyncWrap::MakeCallback", "post hook threw");
+    TryCatch try_catch(env()->isolate());
+    MaybeLocal<Value> ar =
+        post_fn->Call(env()->context(), context, arraysize(vals), vals);
+    if (ar.IsEmpty()) {
+      ClearFatalExceptionHandlers(env());
+      FatalException(env()->isolate(), try_catch);
+      return Local<Value>();
+    }
   }
 
   if (ret.IsEmpty()) {

src/node.cc

@@ -115,6 +115,7 @@ using v8::Integer;
 using v8::Isolate;
 using v8::Local;
 using v8::Locker;
+using v8::MaybeLocal;
 using v8::Message;
 using v8::Number;
 using v8::Object;
@@ -1165,8 +1166,13 @@ Local<Value> MakeCallback(Environment* env,
   }
 
   if (ran_init_callback && !pre_fn.IsEmpty()) {
-    if (pre_fn->Call(object, 0, nullptr).IsEmpty())
-      FatalError("node::MakeCallback", "pre hook threw");
+    TryCatch try_catch(env->isolate());
+    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);
@@ -1177,8 +1183,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, arraysize(vals), vals).IsEmpty())
-      FatalError("node::MakeCallback", "post hook threw");
+    TryCatch try_catch(env->isolate());
+    MaybeLocal<Value> ar =
+        post_fn->Call(env->context(), object, arraysize(vals), vals);
+    if (ar.IsEmpty()) {
+      ClearFatalExceptionHandlers(env);
+      FatalException(env->isolate(), try_catch);
+      return Local<Value>();
+    }
   }
 
   if (ret.IsEmpty()) {
@@ -2352,6 +2364,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

@@ -237,6 +237,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}`);
+    });
+  });
+}