From d45fac1af512a6890d9cf15148d9049ee27dffbd Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Tue, 18 Apr 2017 16:02:50 +0100 Subject: [PATCH] [asyncify] Make errors in callbacks throw globally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If asyncify wraps a function returning promises, the callback will be executed as part of the promise’s `.then()` method. This means that any error thrown from that method will be silenced, and lead to a “unhandled promise rejection” warning in modern engines. Usually, we want these errors to be visible, and even crash the process. --- lib/asyncify.js | 17 +++++++++++++++-- mocha_test/asyncify.js | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/lib/asyncify.js b/lib/asyncify.js index 96eea543b..d65913ffe 100644 --- a/lib/asyncify.js +++ b/lib/asyncify.js @@ -1,5 +1,6 @@ import isObject from 'lodash/isObject'; import initialParams from './internal/initialParams'; +import setImmediate from './internal/setImmediate'; /** * Take a sync function and make it async, passing its return value to a @@ -68,12 +69,24 @@ export default function asyncify(func) { // if result is Promise object if (isObject(result) && typeof result.then === 'function') { result.then(function(value) { - callback(null, value); + invokeCallback(callback, null, value); }, function(err) { - callback(err.message ? err : new Error(err)); + invokeCallback(callback, err.message ? err : new Error(err)); }); } else { callback(null, result); } }); } + +function invokeCallback(callback, error, value) { + try { + callback(error, value); + } catch (e) { + setImmediate(rethrow, e); + } +} + +function rethrow(error) { + throw error; +} diff --git a/mocha_test/asyncify.js b/mocha_test/asyncify.js index a98826cd5..112b8adcb 100644 --- a/mocha_test/asyncify.js +++ b/mocha_test/asyncify.js @@ -92,7 +92,9 @@ describe('asyncify', function(){ }); }); - it('callback error', function(done) { + it('callback error @nodeonly', function(done) { + expectUncaughtException(); + var promisified = function(argument) { return new Promise(function (resolve) { resolve(argument + " resolved"); @@ -105,11 +107,30 @@ describe('asyncify', function(){ throw new Error("error in callback"); } }); + setTimeout(function () { expect(call_count).to.equal(1); done(); }, 15); }); + + it('dont catch errors in the callback @nodeonly', function(done) { + expectUncaughtException(checkErr); + var callbackError = new Error('thrown from callback'); + + function checkErr(err) { + expect(err).to.equal(callbackError); + done(); + } + + function callback() { + throw callbackError; + } + + async.asyncify(function () { + return Promise.reject(new Error('rejection')); + })(callback); + }); } describe('native-promise-only', function() { @@ -134,5 +155,20 @@ describe('asyncify', function(){ var Promise = require('rsvp').Promise; promisifiedTests.call(this, Promise); }); + + function expectUncaughtException(onError) { + // do a weird dance to catch the async thrown error before mocha + var listeners = process.listeners('uncaughtException'); + process.removeAllListeners('uncaughtException'); + process.once('uncaughtException', function onErr(err) { + listeners.forEach(function(listener) { + process.on('uncaughtException', listener); + }); + // can't throw errors in a uncaughtException handler, defer + if (onError) { + setTimeout(onError, 0, err); + } + }); + } }); });