From a32b8fbdef72bb2fc6a821451ff30ff3f9409bbf Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Thu, 11 Feb 2016 13:09:52 -0700 Subject: [PATCH] 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. Ref: https://github.com/nodejs/node/pull/7048 PR-URL: https://github.com/nodejs/node/pull/4507 Reviewed-By: Fedor Indutny --- src/async-wrap.cc | 13 +++++-------- src/node.cc | 13 +++++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index c9f5caad1e4ea8..29ea139f5f91c0 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -207,25 +207,22 @@ Local AsyncWrap::MakeCallback(const Local 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 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 index 8264bfe8bba8a3..b048166cd9d8df 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1165,21 +1165,22 @@ Local 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 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) { @@ -1191,10 +1192,6 @@ Local MakeCallback(Environment* env, } } - if (try_catch.HasCaught()) { - return Undefined(env->isolate()); - } - if (!env->KickNextTick()) return Undefined(env->isolate());