Skip to content

Commit d45fac1

Browse files
committed
[asyncify] Make errors in callbacks throw globally
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.
1 parent 996abc2 commit d45fac1

File tree

2 files changed

+52
-3
lines changed

2 files changed

+52
-3
lines changed

Diff for: lib/asyncify.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import isObject from 'lodash/isObject';
22
import initialParams from './internal/initialParams';
3+
import setImmediate from './internal/setImmediate';
34

45
/**
56
* Take a sync function and make it async, passing its return value to a
@@ -68,12 +69,24 @@ export default function asyncify(func) {
6869
// if result is Promise object
6970
if (isObject(result) && typeof result.then === 'function') {
7071
result.then(function(value) {
71-
callback(null, value);
72+
invokeCallback(callback, null, value);
7273
}, function(err) {
73-
callback(err.message ? err : new Error(err));
74+
invokeCallback(callback, err.message ? err : new Error(err));
7475
});
7576
} else {
7677
callback(null, result);
7778
}
7879
});
7980
}
81+
82+
function invokeCallback(callback, error, value) {
83+
try {
84+
callback(error, value);
85+
} catch (e) {
86+
setImmediate(rethrow, e);
87+
}
88+
}
89+
90+
function rethrow(error) {
91+
throw error;
92+
}

Diff for: mocha_test/asyncify.js

+37-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ describe('asyncify', function(){
9292
});
9393
});
9494

95-
it('callback error', function(done) {
95+
it('callback error @nodeonly', function(done) {
96+
expectUncaughtException();
97+
9698
var promisified = function(argument) {
9799
return new Promise(function (resolve) {
98100
resolve(argument + " resolved");
@@ -105,11 +107,30 @@ describe('asyncify', function(){
105107
throw new Error("error in callback");
106108
}
107109
});
110+
108111
setTimeout(function () {
109112
expect(call_count).to.equal(1);
110113
done();
111114
}, 15);
112115
});
116+
117+
it('dont catch errors in the callback @nodeonly', function(done) {
118+
expectUncaughtException(checkErr);
119+
var callbackError = new Error('thrown from callback');
120+
121+
function checkErr(err) {
122+
expect(err).to.equal(callbackError);
123+
done();
124+
}
125+
126+
function callback() {
127+
throw callbackError;
128+
}
129+
130+
async.asyncify(function () {
131+
return Promise.reject(new Error('rejection'));
132+
})(callback);
133+
});
113134
}
114135

115136
describe('native-promise-only', function() {
@@ -134,5 +155,20 @@ describe('asyncify', function(){
134155
var Promise = require('rsvp').Promise;
135156
promisifiedTests.call(this, Promise);
136157
});
158+
159+
function expectUncaughtException(onError) {
160+
// do a weird dance to catch the async thrown error before mocha
161+
var listeners = process.listeners('uncaughtException');
162+
process.removeAllListeners('uncaughtException');
163+
process.once('uncaughtException', function onErr(err) {
164+
listeners.forEach(function(listener) {
165+
process.on('uncaughtException', listener);
166+
});
167+
// can't throw errors in a uncaughtException handler, defer
168+
if (onError) {
169+
setTimeout(onError, 0, err);
170+
}
171+
});
172+
}
137173
});
138174
});

0 commit comments

Comments
 (0)