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: #7048 PR-URL: #4507 Reviewed-By: Fedor Indutny <fedor@indutny.com>
nodejs · Jul 12, 2016 · 981bbcd · 981bbcd
1 parent 48b7b71
commit 981bbcd
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 41 deletions.
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;
 }