src: fix MakeCallback error handling

Implementations of error handling between node::MakeCallback() and AsyncWrap::MakeCallback() do not return at the same point. Make both executions work the same by moving the early return if there's a caught exception just after the AsyncWrap post callback. Since the domain's call stack is cleared on a caught exception there is no reason to call its exit() callback. Remove the SetVerbose() statement in the AsyncWrap pre/post callback calls since it does not affect the callback call. PR-URL: #4507 Reviewed-By: Fedor Indutny <[email protected]>
nodejs · Feb 15, 2016 · 2cb1594 · 2cb1594
1 parent 35c3832
commit 2cb1594
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 16 deletions.
diff --git a/src/async-wrap.cc b/src/async-wrap.cc
@@ -207,25 +207,22 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   }
 
   if (ran_init_callback() && !pre_fn.IsEmpty()) {
-    try_catch.SetVerbose(false);
     pre_fn->Call(context, 0, nullptr);
     if (try_catch.HasCaught())
       FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
-    try_catch.SetVerbose(true);
   }
 
   Local<Value> ret = cb->Call(context, argc, argv);
 
-  if (try_catch.HasCaught()) {
-    return Undefined(env()->isolate());
-  }
-
   if (ran_init_callback() && !post_fn.IsEmpty()) {
-    try_catch.SetVerbose(false);
     post_fn->Call(context, 0, nullptr);
     if (try_catch.HasCaught())
       FatalError("node::AsyncWrap::MakeCallback", "post hook threw");
-    try_catch.SetVerbose(true);
+  }
+
+  // If the return value is empty then the callback threw.
+  if (ret.IsEmpty()) {
+    return Undefined(env()->isolate());
   }
 
   if (has_domain) {

diff --git a/src/node.cc b/src/node.cc
@@ -1173,21 +1173,22 @@ Local<Value> MakeCallback(Environment* env,
   }
 
   if (ran_init_callback && !pre_fn.IsEmpty()) {
-    try_catch.SetVerbose(false);
     pre_fn->Call(object, 0, nullptr);
     if (try_catch.HasCaught())
       FatalError("node::MakeCallback", "pre hook threw");
-    try_catch.SetVerbose(true);
   }
 
   Local<Value> ret = callback->Call(recv, argc, argv);
 
   if (ran_init_callback && !post_fn.IsEmpty()) {
-    try_catch.SetVerbose(false);
     post_fn->Call(object, 0, nullptr);
     if (try_catch.HasCaught())
       FatalError("node::MakeCallback", "post hook threw");
-    try_catch.SetVerbose(true);
+  }
+
+  // If the return value is empty then the callback threw.
+  if (ret.IsEmpty()) {
+    return Undefined(env->isolate());
   }
 
   if (has_domain) {
@@ -1199,10 +1200,6 @@ Local<Value> MakeCallback(Environment* env,
     }
   }
 
-  if (try_catch.HasCaught()) {
-    return Undefined(env->isolate());
-  }
-
   if (!env->KickNextTick())
     return Undefined(env->isolate());