Browse Source

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: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
process-exit-stdio-flushing
Trevor Norris 9 years ago
parent
commit
575b84ec41
  1. 13
      src/async-wrap.cc
  2. 13
      src/node.cc

13
src/async-wrap.cc

@ -207,25 +207,22 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
} }
if (ran_init_callback() && !pre_fn.IsEmpty()) { if (ran_init_callback() && !pre_fn.IsEmpty()) {
try_catch.SetVerbose(false);
pre_fn->Call(context, 0, nullptr); pre_fn->Call(context, 0, nullptr);
if (try_catch.HasCaught()) if (try_catch.HasCaught())
FatalError("node::AsyncWrap::MakeCallback", "pre hook threw"); FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
try_catch.SetVerbose(true);
} }
Local<Value> ret = cb->Call(context, argc, argv); Local<Value> ret = cb->Call(context, argc, argv);
if (try_catch.HasCaught()) {
return Undefined(env()->isolate());
}
if (ran_init_callback() && !post_fn.IsEmpty()) { if (ran_init_callback() && !post_fn.IsEmpty()) {
try_catch.SetVerbose(false);
post_fn->Call(context, 0, nullptr); post_fn->Call(context, 0, nullptr);
if (try_catch.HasCaught()) if (try_catch.HasCaught())
FatalError("node::AsyncWrap::MakeCallback", "post hook threw"); 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) { if (has_domain) {

13
src/node.cc

@ -1173,21 +1173,22 @@ Local<Value> MakeCallback(Environment* env,
} }
if (ran_init_callback && !pre_fn.IsEmpty()) { if (ran_init_callback && !pre_fn.IsEmpty()) {
try_catch.SetVerbose(false);
pre_fn->Call(object, 0, nullptr); pre_fn->Call(object, 0, nullptr);
if (try_catch.HasCaught()) if (try_catch.HasCaught())
FatalError("node::MakeCallback", "pre hook threw"); FatalError("node::MakeCallback", "pre hook threw");
try_catch.SetVerbose(true);
} }
Local<Value> ret = callback->Call(recv, argc, argv); Local<Value> ret = callback->Call(recv, argc, argv);
if (ran_init_callback && !post_fn.IsEmpty()) { if (ran_init_callback && !post_fn.IsEmpty()) {
try_catch.SetVerbose(false);
post_fn->Call(object, 0, nullptr); post_fn->Call(object, 0, nullptr);
if (try_catch.HasCaught()) if (try_catch.HasCaught())
FatalError("node::MakeCallback", "post hook threw"); 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) { if (has_domain) {
@ -1199,10 +1200,6 @@ Local<Value> MakeCallback(Environment* env,
} }
} }
if (try_catch.HasCaught()) {
return Undefined(env->isolate());
}
if (!env->KickNextTick()) if (!env->KickNextTick())
return Undefined(env->isolate()); return Undefined(env->isolate());

Loading…
Cancel
Save