Skip to content

Commit

Permalink
src: fix MakeCallback error handling
Browse files Browse the repository at this point in the history
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: #7048
PR-URL: #4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
trevnorris authored and Myles Borins committed Jul 12, 2016
1 parent da9595f commit 2eb097f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 16 deletions.
13 changes: 5 additions & 8 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
13 changes: 5 additions & 8 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1165,21 +1165,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) {
Expand All @@ -1191,10 +1192,6 @@ Local<Value> MakeCallback(Environment* env,
}
}

if (try_catch.HasCaught()) {
return Undefined(env->isolate());
}

if (!env->KickNextTick())
return Undefined(env->isolate());

Expand Down

0 comments on commit 2eb097f

Please sign in to comment.