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. PR-URL: #4507 Reviewed-By: Fedor Indutny <fedor@indutny.com>
nodejs · AndreasMadsen · Feb 11, 2016 · Jan 5, 2016 · Feb 11, 2016 · Feb 10, 2016
commit a6a738286036ae9d6ee9f61dd528df453da7fc09
diff --git a/src/async-wrap.cc b/src/async-wrap.cc
@@ -196,43 +196,39 @@ 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());
   }
 
   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());
   }
 

diff --git a/src/env.cc b/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
diff --git a/src/node.cc b/src/node.cc
@@ -1154,48 +1154,45 @@ 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());
   }
 
   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;
 }