Skip to content

Commit

Permalink
[asyncify] Make errors in callbacks throw globally
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidaurelio committed Apr 19, 2017
1 parent 996abc2 commit 1620dd1
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
17 changes: 15 additions & 2 deletions lib/asyncify.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
}
36 changes: 36 additions & 0 deletions mocha_test/asyncify.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ describe('asyncify', function(){
});

it('callback error', function(done) {
expectUncaughtException();

var promisified = function(argument) {
return new Promise(function (resolve) {
resolve(argument + " resolved");
Expand All @@ -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', 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() {
Expand All @@ -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);
}
});
}
});
});

0 comments on commit 1620dd1

Please sign in to comment.