Skip to content

Commit f51042c

Browse files
aearlyaearly
committed
Merge pull request #1050 from caolan/waterfall-multiple-callback-defense
Waterfall multiple callback defense
2 parents 0600652 + 3299335 commit f51042c

File tree

3 files changed

+167
-164
lines changed

3 files changed

+167
-164
lines changed

‎lib/waterfall.js‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,31 @@ import noop from 'lodash/noop';
55
import once from 'lodash/once';
66
import rest from 'lodash/rest';
77

8-
import ensureAsync from './ensureAsync';
9-
import iterator from './iterator';
8+
import onlyOnce from './internal/onlyOnce';
109

1110
export default function(tasks, cb) {
1211
cb = once(cb || noop);
1312
if (!isArray(tasks)) return cb(new Error('First argument to waterfall must be an array of functions'));
1413
if (!tasks.length) return cb();
14+
var taskIndex = 0;
1515

16-
function wrapIterator(iterator) {
17-
return rest(function(err, args) {
16+
function nextTask(args) {
17+
if (taskIndex === tasks.length) {
18+
return cb(null, ...args);
19+
}
20+
21+
var taskCallback = onlyOnce(rest(function(err, args) {
1822
if (err) {
19-
cb.apply(null, [err].concat(args));
20-
} else {
21-
var next = iterator.next();
22-
if (next) {
23-
args.push(wrapIterator(next));
24-
} else {
25-
args.push(cb);
26-
}
27-
ensureAsync(iterator).apply(null, args);
23+
return cb(err, ...args);
2824
}
29-
});
25+
nextTask(args);
26+
}));
27+
28+
args.push(taskCallback);
29+
30+
var task = tasks[taskIndex++];
31+
task(...args);
3032
}
31-
wrapIterator(iterator(tasks))();
33+
34+
nextTask([]);
3235
}

‎mocha_test/waterfall.js‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
var async = require('../lib');
2+
var expect = require('chai').expect;
3+
4+
describe("waterfall", function () {
5+
6+
it('basics', function(done){
7+
var call_order = [];
8+
async.waterfall([
9+
function(callback){
10+
call_order.push('fn1');
11+
setTimeout(function(){callback(null, 'one', 'two');}, 0);
12+
},
13+
function(arg1, arg2, callback){
14+
call_order.push('fn2');
15+
expect(arg1).to.equal('one');
16+
expect(arg2).to.equal('two');
17+
setTimeout(function(){callback(null, arg1, arg2, 'three');}, 25);
18+
},
19+
function(arg1, arg2, arg3, callback){
20+
call_order.push('fn3');
21+
expect(arg1).to.equal('one');
22+
expect(arg2).to.equal('two');
23+
expect(arg3).to.equal('three');
24+
callback(null, 'four');
25+
},
26+
function(arg4, callback){
27+
call_order.push('fn4');
28+
expect(call_order).to.eql(['fn1','fn2','fn3','fn4']);
29+
callback(null, 'test');
30+
}
31+
], function(err){
32+
expect(err === null, err + " passed instead of 'null'");
33+
done();
34+
});
35+
});
36+
37+
it('empty array', function(done){
38+
async.waterfall([], function(err){
39+
if (err) throw err;
40+
done();
41+
});
42+
});
43+
44+
it('non-array', function(done){
45+
async.waterfall({}, function(err){
46+
expect(err.message).to.equal('First argument to waterfall must be an array of functions');
47+
done();
48+
});
49+
});
50+
51+
it('no callback', function(done){
52+
async.waterfall([
53+
function(callback){callback();},
54+
function(callback){callback(); done();}
55+
]);
56+
});
57+
58+
it('async', function(done){
59+
var call_order = [];
60+
async.waterfall([
61+
function(callback){
62+
call_order.push(1);
63+
callback();
64+
call_order.push(2);
65+
},
66+
function(callback){
67+
call_order.push(3);
68+
callback();
69+
},
70+
function(){
71+
expect(call_order).to.eql([1,3]);
72+
done();
73+
}
74+
]);
75+
});
76+
77+
it('error', function(done){
78+
async.waterfall([
79+
function(callback){
80+
callback('error');
81+
},
82+
function(callback){
83+
test.ok(false, 'next function should not be called');
84+
callback();
85+
}
86+
], function(err){
87+
expect(err).to.equal('error');
88+
done();
89+
});
90+
});
91+
92+
it('multiple callback calls', function(){
93+
var arr = [
94+
function(callback){
95+
// call the callback twice. this should call function 2 twice
96+
callback(null, 'one', 'two');
97+
callback(null, 'one', 'two');
98+
},
99+
function(arg1, arg2, callback){
100+
callback(null, arg1, arg2, 'three');
101+
}
102+
];
103+
expect(function () {
104+
async.waterfall(arr, function () {});
105+
}).to.throw(/already called/);
106+
});
107+
108+
it('call in another context', function(done) {
109+
if (process.browser) {
110+
// node only test
111+
done();
112+
return;
113+
}
114+
115+
var vm = require('vm');
116+
var sandbox = {
117+
async: async,
118+
done: done
119+
};
120+
121+
var fn = "(" + (function () {
122+
async.waterfall([function (callback) {
123+
callback();
124+
}], function (err) {
125+
if (err) {
126+
return done(err);
127+
}
128+
done();
129+
});
130+
}).toString() + "())";
131+
132+
vm.runInNewContext(fn, sandbox);
133+
});
134+
135+
it('should not use unnecessary deferrals', function (done) {
136+
var sameStack = true;
137+
138+
async.waterfall([
139+
function (cb) { cb(null, 1); },
140+
function (arg, cb) { cb(); }
141+
], function() {
142+
expect(sameStack).to.equal(true);
143+
done();
144+
});
145+
146+
sameStack = false;
147+
});
148+
});

‎test/test-async.js‎

Lines changed: 1 addition & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* NOTE: We are in the process of migrating these tests to Mocha. If you are
3-
* adding a new test, consider creating a new spec file in mocha_tests/
3+
* adding a new test, please create a new spec file in mocha_tests/
44
*/
55

66
require('babel-core/register');
@@ -392,154 +392,6 @@ exports['retry as an embedded task with interval'] = function(test) {
392392
});
393393
};
394394

395-
exports['waterfall'] = {
396-
397-
'basic': function(test){
398-
test.expect(7);
399-
var call_order = [];
400-
async.waterfall([
401-
function(callback){
402-
call_order.push('fn1');
403-
setTimeout(function(){callback(null, 'one', 'two');}, 0);
404-
},
405-
function(arg1, arg2, callback){
406-
call_order.push('fn2');
407-
test.equals(arg1, 'one');
408-
test.equals(arg2, 'two');
409-
setTimeout(function(){callback(null, arg1, arg2, 'three');}, 25);
410-
},
411-
function(arg1, arg2, arg3, callback){
412-
call_order.push('fn3');
413-
test.equals(arg1, 'one');
414-
test.equals(arg2, 'two');
415-
test.equals(arg3, 'three');
416-
callback(null, 'four');
417-
},
418-
function(arg4, callback){
419-
call_order.push('fn4');
420-
test.same(call_order, ['fn1','fn2','fn3','fn4']);
421-
callback(null, 'test');
422-
}
423-
], function(err){
424-
test.ok(err === null, err + " passed instead of 'null'");
425-
test.done();
426-
});
427-
},
428-
429-
'empty array': function(test){
430-
async.waterfall([], function(err){
431-
if (err) throw err;
432-
test.done();
433-
});
434-
},
435-
436-
'non-array': function(test){
437-
async.waterfall({}, function(err){
438-
test.equals(err.message, 'First argument to waterfall must be an array of functions');
439-
test.done();
440-
});
441-
},
442-
443-
'no callback': function(test){
444-
async.waterfall([
445-
function(callback){callback();},
446-
function(callback){callback(); test.done();}
447-
]);
448-
},
449-
450-
'async': function(test){
451-
var call_order = [];
452-
async.waterfall([
453-
function(callback){
454-
call_order.push(1);
455-
callback();
456-
call_order.push(2);
457-
},
458-
function(callback){
459-
call_order.push(3);
460-
callback();
461-
},
462-
function(){
463-
test.same(call_order, [1,2,3]);
464-
test.done();
465-
}
466-
]);
467-
},
468-
469-
'error': function(test){
470-
test.expect(1);
471-
async.waterfall([
472-
function(callback){
473-
callback('error');
474-
},
475-
function(callback){
476-
test.ok(false, 'next function should not be called');
477-
callback();
478-
}
479-
], function(err){
480-
test.equals(err, 'error');
481-
});
482-
setTimeout(test.done, 50);
483-
},
484-
485-
'multiple callback calls': function(test){
486-
var call_order = [];
487-
var arr = [
488-
function(callback){
489-
call_order.push(1);
490-
// call the callback twice. this should call function 2 twice
491-
callback(null, 'one', 'two');
492-
callback(null, 'one', 'two');
493-
},
494-
function(arg1, arg2, callback){
495-
call_order.push(2);
496-
callback(null, arg1, arg2, 'three');
497-
},
498-
function(arg1, arg2, arg3, callback){
499-
call_order.push(3);
500-
callback(null, 'four');
501-
},
502-
function(/*arg4*/){
503-
call_order.push(4);
504-
arr[3] = function(){
505-
call_order.push(4);
506-
test.same(call_order, [1,2,2,3,3,4,4]);
507-
test.done();
508-
};
509-
}
510-
];
511-
async.waterfall(arr);
512-
},
513-
514-
'call in another context': function(test) {
515-
if (isBrowser()) {
516-
// node only test
517-
test.done();
518-
return;
519-
}
520-
521-
var vm = require('vm');
522-
var sandbox = {
523-
async: async,
524-
test: test
525-
};
526-
527-
var fn = "(" + (function () {
528-
async.waterfall([function (callback) {
529-
callback();
530-
}], function (err) {
531-
if (err) {
532-
return test.done(err);
533-
}
534-
test.done();
535-
});
536-
}).toString() + "())";
537-
538-
vm.runInNewContext(fn, sandbox);
539-
}
540-
541-
};
542-
543395
exports['parallel'] = function(test){
544396
var call_order = [];
545397
async.parallel([

0 commit comments

Comments
 (0)