src: remove TryCatch in MakeCallback

After attempting to use ReThrow() and Reset() there were cases where
firing the domain's error handlers was not happening. Or in some cases
reentering MakeCallback would still cause the domain enter callback to
abort (because the error had not been Reset yet).

In order for the script to properly stop execution when a subsequent
call to MakeCallback throws it must not be located within a TryCatch.

Ref: https://github.com/nodejs/node/pull/7048
PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>

src/async-wrap.cc

@@ -196,33 +196,28 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
     }
   }
 
-  TryCatch try_catch(env()->isolate());
-  try_catch.SetVerbose(true);
-
   if (has_domain) {
     Local<Value> enter_v = domain->Get(env()->enter_string());
     if (enter_v->IsFunction()) {
-      enter_v.As<Function>()->Call(domain, 0, nullptr);
-      if (try_catch.HasCaught())
-        return Undefined(env()->isolate());
+      if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
+        FatalError("node::AsyncWrap::MakeCallback",
+                   "domain enter callback threw, please report this");
+      }
     }
   }
 
   if (ran_init_callback() && !pre_fn.IsEmpty()) {
-    pre_fn->Call(context, 0, nullptr);
-    if (try_catch.HasCaught())
+    if (pre_fn->Call(context, 0, nullptr).IsEmpty())
       FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
   }
 
   Local<Value> ret = cb->Call(context, argc, argv);
 
   if (ran_init_callback() && !post_fn.IsEmpty()) {
-    post_fn->Call(context, 0, nullptr);
-    if (try_catch.HasCaught())
+    if (post_fn->Call(context, 0, nullptr).IsEmpty())
       FatalError("node::AsyncWrap::MakeCallback", "post hook threw");
   }
 
-  // If the return value is empty then the callback threw.
   if (ret.IsEmpty()) {
     return Undefined(env()->isolate());
   }
@@ -230,9 +225,10 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   if (has_domain) {
     Local<Value> exit_v = domain->Get(env()->exit_string());
     if (exit_v->IsFunction()) {
-      exit_v.As<Function>()->Call(domain, 0, nullptr);
-      if (try_catch.HasCaught())
-        return Undefined(env()->isolate());
+      if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
+        FatalError("node::AsyncWrap::MakeCallback",
+                   "domain exit callback threw, please report this");
+      }
     }
   }
 
@@ -251,9 +247,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
     return ret;
   }
 
-  env()->tick_callback_function()->Call(process, 0, nullptr);
-
-  if (try_catch.HasCaught()) {
+  if (env()->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
     return Undefined(env()->isolate());
   }
 

src/env.cc

@@ -11,6 +11,7 @@ using v8::Message;
 using v8::StackFrame;
 using v8::StackTrace;
 using v8::TryCatch;
+using v8::Value;
 
 void Environment::PrintSyncTrace() const {
   if (!trace_sync_io_)
@@ -73,16 +74,10 @@ bool Environment::KickNextTick(Environment::AsyncCallbackScope* scope) {
     return true;
   }
 
-  // process nextTicks after call
-  TryCatch try_catch(isolate());
-  try_catch.SetVerbose(true);
-  tick_callback_function()->Call(process_object(), 0, nullptr);
+  Local<Value> ret =
+    tick_callback_function()->Call(process_object(), 0, nullptr);
 
-  if (try_catch.HasCaught()) {
-    return false;
-  }
-
-  return true;
+  return !ret.IsEmpty();
 }
 
 }  // namespace node

src/node.cc

@@ -1154,33 +1154,28 @@ Local<Value> MakeCallback(Environment* env,
     }
   }
 
-  TryCatch try_catch(env->isolate());
-  try_catch.SetVerbose(true);
-
   if (has_domain) {
     Local<Value> enter_v = domain->Get(env->enter_string());
     if (enter_v->IsFunction()) {
-        enter_v.As<Function>()->Call(domain, 0, nullptr);
-      if (try_catch.HasCaught())
-        return Undefined(env->isolate());
+      if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
+        FatalError("node::MakeCallback",
+                   "domain enter callback threw, please report this");
+      }
     }
   }
 
   if (ran_init_callback && !pre_fn.IsEmpty()) {
-    pre_fn->Call(object, 0, nullptr);
-    if (try_catch.HasCaught())
+    if (pre_fn->Call(object, 0, nullptr).IsEmpty())
       FatalError("node::MakeCallback", "pre hook threw");
   }
 
   Local<Value> ret = callback->Call(recv, argc, argv);
 
   if (ran_init_callback && !post_fn.IsEmpty()) {
-    post_fn->Call(object, 0, nullptr);
-    if (try_catch.HasCaught())
+    if (post_fn->Call(object, 0, nullptr).IsEmpty())
       FatalError("node::MakeCallback", "post hook threw");
   }
 
-  // If the return value is empty then the callback threw.
   if (ret.IsEmpty()) {
     return Undefined(env->isolate());
   }
@@ -1188,14 +1183,16 @@ Local<Value> MakeCallback(Environment* env,
   if (has_domain) {
     Local<Value> exit_v = domain->Get(env->exit_string());
     if (exit_v->IsFunction()) {
-      exit_v.As<Function>()->Call(domain, 0, nullptr);
-      if (try_catch.HasCaught())
-        return Undefined(env->isolate());
+      if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
+        FatalError("node::MakeCallback",
+                   "domain exit callback threw, please report this");
+      }
     }
   }
 
-  if (!env->KickNextTick(&callback_scope))
+  if (!env->KickNextTick(&callback_scope)) {
     return Undefined(env->isolate());
+  }
 
   return ret;
 }