From 6f16882733f768a5ebcd5bfe100d1bfc75caf99c Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Mon, 22 Feb 2016 22:50:56 -0700 Subject: [PATCH] async_wrap: notify post if intercepted exception The second argument of the post callback is a boolean indicating whether the callback threw and was intercepted by uncaughtException or a domain. Currently node::MakeCallback has no way of retrieving a uid for the object. This is coming in a future patch. PR-URL: https://github.com/nodejs/node/pull/5756 Reviewed-By: Ben Noordhuis Reviewed-By: Andreas Madsen --- src/async-wrap.cc | 5 ++- src/node.cc | 7 +++- .../test-async-wrap-post-did-throw.js | 34 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-async-wrap-post-did-throw.js diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 570ed2f165b2ba..db9d0a4f354015 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -9,6 +9,7 @@ #include "v8-profiler.h" using v8::Array; +using v8::Boolean; using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; @@ -231,7 +232,9 @@ Local AsyncWrap::MakeCallback(const Local cb, Local ret = cb->Call(context, argc, argv); if (ran_init_callback() && !post_fn.IsEmpty()) { - if (post_fn->Call(context, 1, &uid).IsEmpty()) + Local did_throw = Boolean::New(env()->isolate(), ret.IsEmpty()); + Local vals[] = { uid, did_throw }; + if (post_fn->Call(context, ARRAY_SIZE(vals), vals).IsEmpty()) FatalError("node::AsyncWrap::MakeCallback", "post hook threw"); } diff --git a/src/node.cc b/src/node.cc index e869a9ee724f04..1bd995d789fc87 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1182,7 +1182,12 @@ Local MakeCallback(Environment* env, Local ret = callback->Call(recv, argc, argv); if (ran_init_callback && !post_fn.IsEmpty()) { - if (post_fn->Call(object, 0, nullptr).IsEmpty()) + Local did_throw = Boolean::New(env->isolate(), ret.IsEmpty()); + // Currently there's no way to retrieve an uid from node::MakeCallback(). + // This needs to be fixed. + Local vals[] = + { Undefined(env->isolate()).As(), did_throw }; + if (post_fn->Call(object, ARRAY_SIZE(vals), vals).IsEmpty()) FatalError("node::MakeCallback", "post hook threw"); } diff --git a/test/parallel/test-async-wrap-post-did-throw.js b/test/parallel/test-async-wrap-post-did-throw.js new file mode 100644 index 00000000000000..9781983f58987e --- /dev/null +++ b/test/parallel/test-async-wrap-post-did-throw.js @@ -0,0 +1,34 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const async_wrap = process.binding('async_wrap'); +var asyncThrows = 0; +var uncaughtExceptionCount = 0; + +process.on('uncaughtException', (e) => { + assert.equal(e.message, 'oh noes!', 'error messages do not match'); +}); + +process.on('exit', () => { + process.removeAllListeners('uncaughtException'); + assert.equal(uncaughtExceptionCount, 1); + assert.equal(uncaughtExceptionCount, asyncThrows); +}); + +function init() { } +function post(id, threw) { + if (threw) + uncaughtExceptionCount++; +} + +async_wrap.setupHooks({ init, post }); +async_wrap.enable(); + +// Timers still aren't supported, so use crypto API. +// It's also important that the callback not happen in a nextTick, like many +// error events in core. +require('crypto').randomBytes(0, () => { + asyncThrows++; + throw new Error('oh noes!'); +});