Merge pull request #1050 from caolan/waterfall-multiple-callback-defense

Waterfall multiple callback defense

3 files changed, 167 insertions, 164 deletions

diff --git a/lib/waterfall.js b/lib/waterfall.js
index 8867c87..a31b3b4 100644
--- a/lib/waterfall.js
+++ b/lib/waterfall.js
@@ -5,28 +5,31 @@ import noop from 'lodash/noop';
 import once from 'lodash/once';
 import rest from 'lodash/rest';
 
-import ensureAsync from './ensureAsync';
-import iterator from './iterator';
+import onlyOnce from './internal/onlyOnce';
 
 export default  function(tasks, cb) {
     cb = once(cb || noop);
     if (!isArray(tasks)) return cb(new Error('First argument to waterfall must be an array of functions'));
     if (!tasks.length) return cb();
+    var taskIndex = 0;
 
-    function wrapIterator(iterator) {
-        return rest(function(err, args) {
+    function nextTask(args) {
+        if (taskIndex === tasks.length) {
+            return cb(null, ...args);
+        }
+
+        var taskCallback = onlyOnce(rest(function(err, args) {
             if (err) {
-                cb.apply(null, [err].concat(args));
-            } else {
-                var next = iterator.next();
-                if (next) {
-                    args.push(wrapIterator(next));
-                } else {
-                    args.push(cb);
-                }
-                ensureAsync(iterator).apply(null, args);
+                return cb(err, ...args);
             }
-        });
+            nextTask(args);
+        }));
+
+        args.push(taskCallback);
+
+        var task = tasks[taskIndex++];
+        task(...args);
     }
-    wrapIterator(iterator(tasks))();
+
+    nextTask([]);
 }
diff --git a/mocha_test/waterfall.js b/mocha_test/waterfall.js
new file mode 100644
index 0000000..b1d1561
--- /dev/null
+++ b/mocha_test/waterfall.js
@@ -0,0 +1,148 @@
+var async = require('../lib');
+var expect = require('chai').expect;
+
+describe("waterfall", function () {
+
+    it('basics', function(done){
+        var call_order = [];
+        async.waterfall([
+            function(callback){
+                call_order.push('fn1');
+                setTimeout(function(){callback(null, 'one', 'two');}, 0);
+            },
+            function(arg1, arg2, callback){
+                call_order.push('fn2');
+                expect(arg1).to.equal('one');
+                expect(arg2).to.equal('two');
+                setTimeout(function(){callback(null, arg1, arg2, 'three');}, 25);
+            },
+            function(arg1, arg2, arg3, callback){
+                call_order.push('fn3');
+                expect(arg1).to.equal('one');
+                expect(arg2).to.equal('two');
+                expect(arg3).to.equal('three');
+                callback(null, 'four');
+            },
+            function(arg4, callback){
+                call_order.push('fn4');
+                expect(call_order).to.eql(['fn1','fn2','fn3','fn4']);
+                callback(null, 'test');
+            }
+        ], function(err){
+            expect(err === null, err + " passed instead of 'null'");
+            done();
+        });
+    });
+
+    it('empty array', function(done){
+        async.waterfall([], function(err){
+            if (err) throw err;
+            done();
+        });
+    });
+
+    it('non-array', function(done){
+        async.waterfall({}, function(err){
+            expect(err.message).to.equal('First argument to waterfall must be an array of functions');
+            done();
+        });
+    });
+
+    it('no callback', function(done){
+        async.waterfall([
+            function(callback){callback();},
+            function(callback){callback(); done();}
+        ]);
+    });
+
+    it('async', function(done){
+        var call_order = [];
+        async.waterfall([
+            function(callback){
+                call_order.push(1);
+                callback();
+                call_order.push(2);
+            },
+            function(callback){
+                call_order.push(3);
+                callback();
+            },
+            function(){
+                expect(call_order).to.eql([1,3]);
+                done();
+            }
+        ]);
+    });
+
+    it('error', function(done){
+        async.waterfall([
+            function(callback){
+                callback('error');
+            },
+            function(callback){
+                test.ok(false, 'next function should not be called');
+                callback();
+            }
+        ], function(err){
+            expect(err).to.equal('error');
+            done();
+        });
+    });
+
+    it('multiple callback calls', function(){
+        var arr = [
+            function(callback){
+                // call the callback twice. this should call function 2 twice
+                callback(null, 'one', 'two');
+                callback(null, 'one', 'two');
+            },
+            function(arg1, arg2, callback){
+                callback(null, arg1, arg2, 'three');
+            }
+        ];
+        expect(function () {
+            async.waterfall(arr, function () {});
+        }).to.throw(/already called/);
+    });
+
+    it('call in another context', function(done) {
+        if (process.browser) {
+            // node only test
+            done();
+            return;
+        }
+
+        var vm = require('vm');
+        var sandbox = {
+            async: async,
+            done: done
+        };
+
+        var fn = "(" + (function () {
+            async.waterfall([function (callback) {
+                callback();
+            }], function (err) {
+                if (err) {
+                    return done(err);
+                }
+                done();
+            });
+        }).toString() + "())";
+
+        vm.runInNewContext(fn, sandbox);
+    });
+
+    it('should not use unnecessary deferrals', function (done) {
+        var sameStack = true;
+
+        async.waterfall([
+            function (cb) { cb(null, 1); },
+            function (arg, cb) { cb(); }
+        ], function() {
+            expect(sameStack).to.equal(true);
+            done();
+        });
+
+        sameStack = false;
+    });
+});
diff --git a/test/test-async.js b/test/test-async.js
index b4aef6c..85e9f0e 100755
--- a/test/test-async.js
+++ b/test/test-async.js
@@ -1,6 +1,6 @@
 /**
  * NOTE:  We are in the process of migrating these tests to Mocha.  If you are
- * adding a new test, consider creating a new spec file in mocha_tests/
+ * adding a new test, please create a new spec file in mocha_tests/
  */
 
 require('babel-core/register');
@@ -392,154 +392,6 @@ exports['retry as an embedded task with interval'] = function(test) {
     });
 };
 
-exports['waterfall'] = {
-
-    'basic': function(test){
-    test.expect(7);
-    var call_order = [];
-    async.waterfall([
-        function(callback){
-            call_order.push('fn1');
-            setTimeout(function(){callback(null, 'one', 'two');}, 0);
-        },
-        function(arg1, arg2, callback){
-            call_order.push('fn2');
-            test.equals(arg1, 'one');
-            test.equals(arg2, 'two');
-            setTimeout(function(){callback(null, arg1, arg2, 'three');}, 25);
-        },
-        function(arg1, arg2, arg3, callback){
-            call_order.push('fn3');
-            test.equals(arg1, 'one');
-            test.equals(arg2, 'two');
-            test.equals(arg3, 'three');
-            callback(null, 'four');
-        },
-        function(arg4, callback){
-            call_order.push('fn4');
-            test.same(call_order, ['fn1','fn2','fn3','fn4']);
-            callback(null, 'test');
-        }
-    ], function(err){
-        test.ok(err === null, err + " passed instead of 'null'");
-        test.done();
-    });
-},
-
-    'empty array': function(test){
-    async.waterfall([], function(err){
-        if (err) throw err;
-        test.done();
-    });
-},
-
-    'non-array': function(test){
-    async.waterfall({}, function(err){
-        test.equals(err.message, 'First argument to waterfall must be an array of functions');
-        test.done();
-    });
-},
-
-    'no callback': function(test){
-    async.waterfall([
-        function(callback){callback();},
-        function(callback){callback(); test.done();}
-    ]);
-},
-
-    'async': function(test){
-    var call_order = [];
-    async.waterfall([
-        function(callback){
-            call_order.push(1);
-            callback();
-            call_order.push(2);
-        },
-        function(callback){
-            call_order.push(3);
-            callback();
-        },
-        function(){
-            test.same(call_order, [1,2,3]);
-            test.done();
-        }
-    ]);
-},
-
-    'error': function(test){
-    test.expect(1);
-    async.waterfall([
-        function(callback){
-            callback('error');
-        },
-        function(callback){
-            test.ok(false, 'next function should not be called');
-            callback();
-        }
-    ], function(err){
-        test.equals(err, 'error');
-    });
-    setTimeout(test.done, 50);
-},
-
-    'multiple callback calls': function(test){
-    var call_order = [];
-    var arr = [
-        function(callback){
-            call_order.push(1);
-            // call the callback twice. this should call function 2 twice
-            callback(null, 'one', 'two');
-            callback(null, 'one', 'two');
-        },
-        function(arg1, arg2, callback){
-            call_order.push(2);
-            callback(null, arg1, arg2, 'three');
-        },
-        function(arg1, arg2, arg3, callback){
-            call_order.push(3);
-            callback(null, 'four');
-        },
-        function(/*arg4*/){
-            call_order.push(4);
-            arr[3] = function(){
-                call_order.push(4);
-                test.same(call_order, [1,2,2,3,3,4,4]);
-                test.done();
-            };
-        }
-    ];
-    async.waterfall(arr);
-},
-
-    'call in another context': function(test) {
-    if (isBrowser()) {
-        // node only test
-        test.done();
-        return;
-    }
-
-    var vm = require('vm');
-    var sandbox = {
-        async: async,
-        test: test
-    };
-
-    var fn = "(" + (function () {
-        async.waterfall([function (callback) {
-            callback();
-        }], function (err) {
-            if (err) {
-                return test.done(err);
-            }
-            test.done();
-        });
-    }).toString() + "())";
-
-    vm.runInNewContext(fn, sandbox);
-}
-
-};
-
 exports['parallel'] = function(test){
     var call_order = [];
     async.parallel([