Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Waterfall multiple callback defense #1050

Merged
merged 3 commits into from
Mar 8, 2016
Merged

Waterfall multiple callback defense #1050

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a201c6e
convert test to mocha
aearly Mar 7, 2016
d2fe2b3
defend against multiple callbacks
aearly Mar 7, 2016
3299335
clean up code, use ES6
aearly Mar 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 19 additions & 15 deletions lib/waterfall.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,32 @@ 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.apply(null, [null].concat(args));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's being transpiled so you can do cb(null, ...args)

}
var task = tasks[taskIndex];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do the postfix here tasks[taskIndex++]

taskIndex++;

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.apply(null, [err].concat(args));
}
});
nextTask(args);
}));

args.push(taskCallback);

task.apply(null, args);
}
wrapIterator(iterator(tasks))();

nextTask([]);
}
148 changes: 148 additions & 0 deletions mocha_test/waterfall.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
});
});
150 changes: 1 addition & 149 deletions test/test-async.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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([
Expand Down