Merge pull request #1408 from davidaurelio/master

[asyncify] Make errors in callbacks throw globally
caolan · Apr 22, 2017 · dd7cf79 · dd7cf79
2 parents eb95a0a + d45fac1
commit dd7cf79
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 3 deletions.
diff --git 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
@@ -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);
+                }
+            });
+        }
     });
 });