From 6d61acf4fb5e6f7ecee7e012ccd068b7d3a51616 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 3 Jun 2018 17:00:44 -0700 Subject: [PATCH 01/14] cancelable foreach --- lib/internal/eachOfLimit.js | 10 ++++-- test/eachOf.js | 65 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/lib/internal/eachOfLimit.js b/lib/internal/eachOfLimit.js index 6ce076252..6ca34aeed 100644 --- a/lib/internal/eachOfLimit.js +++ b/lib/internal/eachOfLimit.js @@ -14,17 +14,23 @@ export default function _eachOfLimit(limit) { } var nextElem = iterator(obj); var done = false; + var canceled = false; var running = 0; var looping = false; function iterateeCallback(err, value) { running -= 1; - if (err) { + if (err && !canceled) { done = true; callback(err); } + else if (err === false) { + done = true; + canceled = true; + } else if (value === breakLoop || (done && running <= 0)) { done = true; + if (canceled) return return callback(null); } else if (!looping) { @@ -38,7 +44,7 @@ export default function _eachOfLimit(limit) { var elem = nextElem(); if (elem === null) { done = true; - if (running <= 0) { + if (running <= 0 && !canceled) { callback(null); } return; diff --git a/test/eachOf.js b/test/eachOf.js index dd73c22fd..6ded04f73 100644 --- a/test/eachOf.js +++ b/test/eachOf.js @@ -403,4 +403,69 @@ describe("eachOf", function() { done(); }); }); + + it('forEachOfLimit canceled', function(done) { + var obj = { a: 1, b: 2, c: 3, d: 4, e: 5 }; + var call_order = []; + + async.forEachOfLimit(obj, 3, function(value, key, callback){ + call_order.push(value, key); + if (value === 2) { + return callback(false); + } + callback() + }, function(err){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([ 1, "a", 2, "b" ]); + done() + }, 10); + }); + + it('forEachOfLimit canceled (async)', function(done) { + var obj = { a: 1, b: 2, c: 3, d: 4, e: 5 }; + var call_order = []; + + async.forEachOfLimit(obj, 3, function(value, key, callback){ + call_order.push(value, key); + setTimeout(() => { + if (value === 2) { + return callback(false); + } + callback() + }) + }, function(err){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([ 1, "a", 2, "b", 3, "c", 4, "d" ]); + done() + }, 20); + }); + + it('forEachOfLimit canceled (async, w/ error)', function(done) { + var obj = { a: 1, b: 2, c: 3, d: 4, e: 5 }; + var call_order = []; + + async.forEachOfLimit(obj, 3, function(value, key, callback){ + call_order.push(value, key); + setTimeout(() => { + if (value === 2) { + return callback(false); + } + if (value === 3) { + return callback('fail'); + } + callback() + }) + }, function(err){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([ 1, "a", 2, "b", 3, "c", 4, "d" ]); + done() + }, 20); + }); + }); From e56fea9f6f69ab03161a3b33a7369cdab3117134 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 3 Jun 2018 17:44:18 -0700 Subject: [PATCH 02/14] cancelable waterfall --- lib/waterfall.js | 1 + test/parallel.js | 26 ++++++++++++++++++++++++++ test/waterfall.js | 22 ++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/lib/waterfall.js b/lib/waterfall.js index 3e71d2cec..7f5c12e6d 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -75,6 +75,7 @@ export default function(tasks, callback) { } function next(err/*, ...args*/) { + if (err === null) return // canceled if (err || taskIndex === tasks.length) { return callback.apply(null, arguments); } diff --git a/test/parallel.js b/test/parallel.js index 2b07822c6..e4af6f86d 100644 --- a/test/parallel.js +++ b/test/parallel.js @@ -191,6 +191,32 @@ describe('parallel', function() { }); }); + it('parallel limit canceled', function(done) { + const call_order = [] + async.parallelLimit([ + function(callback){ + call_order.push(1) + callback(); + }, + function(callback){ + call_order.push(2) + callback(false); + }, + function(callback){ + call_order.push(3) + callback('error', 2); + } + ], + 1, + function(err){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([1, 2]); + done() + }, 25); + }); + it('parallel call in another context @nycinvalid @nodeonly', function(done) { var vm = require('vm'); var sandbox = { diff --git a/test/waterfall.js b/test/waterfall.js index 78b11bbb6..901d59b0e 100644 --- a/test/waterfall.js +++ b/test/waterfall.js @@ -90,6 +90,28 @@ describe("waterfall", function () { }); }); + + it('canceled', function(done){ + const call_order = [] + async.waterfall([ + function(callback){ + call_order.push(1) + callback(null); + }, + function(callback){ + call_order.push(2) + assert(false, 'next function should not be called'); + callback(); + } + ], function(err){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([1]) + done() + }, 10) + }); + it('multiple callback calls', function(){ var arr = [ function(callback){ From 365f19c1916a461c430e58a51ffe91ae6e964f59 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 3 Jun 2018 17:55:26 -0700 Subject: [PATCH 03/14] cancellable auto --- lib/auto.js | 9 +++++++-- test/auto.js | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/auto.js b/lib/auto.js index a6150aae6..0175fda7b 100644 --- a/lib/auto.js +++ b/lib/auto.js @@ -102,6 +102,7 @@ export default function (tasks, concurrency, callback) { var results = {}; var runningTasks = 0; + var canceled = false; var hasError = false; var listeners = Object.create(null); @@ -156,7 +157,7 @@ export default function (tasks, concurrency, callback) { } function processQueue() { - if (readyTasks.length === 0 && runningTasks === 0) { + if (readyTasks.length === 0 && runningTasks === 0 && !canceled) { return callback(null, results); } while(readyTasks.length && runningTasks < concurrency) { @@ -189,6 +190,10 @@ export default function (tasks, concurrency, callback) { var taskCallback = onlyOnce(function(err, result) { runningTasks--; + if (err === false) { + canceled = true + return + } if (arguments.length > 2) { result = slice(arguments, 1); } @@ -200,7 +205,7 @@ export default function (tasks, concurrency, callback) { safeResults[key] = result; hasError = true; listeners = Object.create(null); - + if (canceled) return callback(err, safeResults); } else { results[key] = result; diff --git a/test/auto.js b/test/auto.js index 489dbea0c..1c901359c 100644 --- a/test/auto.js +++ b/test/auto.js @@ -168,6 +168,31 @@ describe('auto', function () { setTimeout(done, 100); }); + it('auto canceled', function(done){ + const call_order = [] + async.auto({ + task1: function(callback){ + call_order.push(1) + callback(false); + }, + task2: ['task1', function(/*results, callback*/){ + call_order.push(2) + throw new Error('task2 should not be called'); + }], + task3: function(callback){ + call_order.push(3) + callback('testerror2'); + } + }, + function(err){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([1, 3]) + done() + }, 10); + }); + it('auto no callback', function(done){ async.auto({ task1: function(callback){callback();}, From 8fedfa3dfc72490e1fc99eb8b9b0c742ffed6e98 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 3 Jun 2018 22:32:12 -0700 Subject: [PATCH 04/14] fix lint --- test/auto.js | 2 +- test/eachOf.js | 6 +++--- test/parallel.js | 2 +- test/waterfall.js | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/auto.js b/test/auto.js index 1c901359c..055cb3183 100644 --- a/test/auto.js +++ b/test/auto.js @@ -184,7 +184,7 @@ describe('auto', function () { callback('testerror2'); } }, - function(err){ + function(){ throw new Error('should not get here') }); setTimeout(() => { diff --git a/test/eachOf.js b/test/eachOf.js index 6ded04f73..791413a4a 100644 --- a/test/eachOf.js +++ b/test/eachOf.js @@ -414,7 +414,7 @@ describe("eachOf", function() { return callback(false); } callback() - }, function(err){ + }, function(){ throw new Error('should not get here') }); setTimeout(() => { @@ -435,7 +435,7 @@ describe("eachOf", function() { } callback() }) - }, function(err){ + }, function(){ throw new Error('should not get here') }); setTimeout(() => { @@ -459,7 +459,7 @@ describe("eachOf", function() { } callback() }) - }, function(err){ + }, function(){ throw new Error('should not get here') }); setTimeout(() => { diff --git a/test/parallel.js b/test/parallel.js index e4af6f86d..c7de8881b 100644 --- a/test/parallel.js +++ b/test/parallel.js @@ -208,7 +208,7 @@ describe('parallel', function() { } ], 1, - function(err){ + function(){ throw new Error('should not get here') }); setTimeout(() => { diff --git a/test/waterfall.js b/test/waterfall.js index 901d59b0e..440cb7a6c 100644 --- a/test/waterfall.js +++ b/test/waterfall.js @@ -103,7 +103,7 @@ describe("waterfall", function () { assert(false, 'next function should not be called'); callback(); } - ], function(err){ + ], function(){ throw new Error('should not get here') }); setTimeout(() => { From dfd21d0e38f5f0e25bd9ccde420782f7d5faf226 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 3 Jun 2018 22:45:28 -0700 Subject: [PATCH 05/14] fix tests --- lib/tryEach.js | 2 +- lib/waterfall.js | 2 +- test/auto.js | 2 +- test/waterfall.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/tryEach.js b/lib/tryEach.js index 87fba1266..82649b4bd 100644 --- a/lib/tryEach.js +++ b/lib/tryEach.js @@ -52,7 +52,7 @@ export default function tryEach(tasks, callback) { result = res; } error = err; - callback(!err); + callback(err ? null : {}); }); }, function () { callback(error, result); diff --git a/lib/waterfall.js b/lib/waterfall.js index 7f5c12e6d..10db92657 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -75,7 +75,7 @@ export default function(tasks, callback) { } function next(err/*, ...args*/) { - if (err === null) return // canceled + if (err === false) return // canceled if (err || taskIndex === tasks.length) { return callback.apply(null, arguments); } diff --git a/test/auto.js b/test/auto.js index 055cb3183..48de743b0 100644 --- a/test/auto.js +++ b/test/auto.js @@ -210,7 +210,7 @@ describe('auto', function () { it('auto error should pass partial results', function(done) { async.auto({ task1: function(callback){ - callback(false, 'result1'); + callback(null, 'result1'); }, task2: ['task1', function(results, callback){ callback('testerror', 'result2'); diff --git a/test/waterfall.js b/test/waterfall.js index 440cb7a6c..8892f041d 100644 --- a/test/waterfall.js +++ b/test/waterfall.js @@ -96,7 +96,7 @@ describe("waterfall", function () { async.waterfall([ function(callback){ call_order.push(1) - callback(null); + callback(false); }, function(callback){ call_order.push(2) From 062908b448f3de98fe89aebf27b404f58e116a60 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 10 Jun 2018 18:39:45 -0700 Subject: [PATCH 06/14] cancelable whilst/until/during/forever --- lib/doDuring.js | 2 ++ lib/doWhilst.js | 1 + lib/during.js | 2 ++ lib/forever.js | 1 + lib/whilst.js | 1 + test/during.js | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ test/forever.js | 13 +++++++++++++ test/until.js | 32 ++++++++++++++++++++++++++++++++ test/whilst.js | 32 ++++++++++++++++++++++++++++++++ 9 files changed, 132 insertions(+) diff --git a/lib/doDuring.js b/lib/doDuring.js index ac2a04c7e..5eb2b9279 100644 --- a/lib/doDuring.js +++ b/lib/doDuring.js @@ -30,6 +30,7 @@ export default function doDuring(fn, test, callback) { function next(err/*, ...args*/) { if (err) return callback(err); + if (err === false) return; var args = slice(arguments, 1); args.push(check); _test.apply(this, args); @@ -37,6 +38,7 @@ export default function doDuring(fn, test, callback) { function check(err, truth) { if (err) return callback(err); + if (err === false) return; if (!truth) return callback(null); _fn(next); } diff --git a/lib/doWhilst.js b/lib/doWhilst.js index 3c2b865a7..524582081 100644 --- a/lib/doWhilst.js +++ b/lib/doWhilst.js @@ -31,6 +31,7 @@ export default function doWhilst(iteratee, test, callback) { var _iteratee = wrapAsync(iteratee); var next = function(err/*, ...args*/) { if (err) return callback(err); + if (err === false) return; var args = slice(arguments, 1); if (test.apply(this, args)) return _iteratee(next); callback.apply(null, [null].concat(args)); diff --git a/lib/during.js b/lib/during.js index cda9979ea..02da045a2 100644 --- a/lib/during.js +++ b/lib/during.js @@ -45,11 +45,13 @@ export default function during(test, fn, callback) { function next(err) { if (err) return callback(err); + if (err === false) return; _test(check); } function check(err, truth) { if (err) return callback(err); + if (err === false) return; if (!truth) return callback(null); _fn(next); } diff --git a/lib/forever.js b/lib/forever.js index 37532513f..fb69adf0c 100644 --- a/lib/forever.js +++ b/lib/forever.js @@ -38,6 +38,7 @@ export default function forever(fn, errback) { function next(err) { if (err) return done(err); + if (err === false) return; task(next); } next(); diff --git a/lib/whilst.js b/lib/whilst.js index da748fada..32c3b52e5 100644 --- a/lib/whilst.js +++ b/lib/whilst.js @@ -44,6 +44,7 @@ export default function whilst(test, iteratee, callback) { if (!test()) return callback(null); var next = function(err/*, ...args*/) { if (err) return callback(err); + if (err === false) return; if (test()) return _iteratee(next); var args = slice(arguments, 1); callback.apply(null, [null].concat(args)); diff --git a/test/during.js b/test/during.js index 13db8b327..e0636e501 100644 --- a/test/during.js +++ b/test/during.js @@ -34,6 +34,22 @@ describe('during', function() { ); }); + it('during canceling', (done) => { + let counter = 0; + async.during( + cb => cb(null, true), + cb => { + counter++ + cb(counter === 2 ? false : null); + }, + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }) + it('doDuring', function(done) { var call_order = []; @@ -95,4 +111,36 @@ describe('during', function() { } ); }); + + it('doDuring canceling', (done) => { + let counter = 0; + async.doDuring( + cb => { + counter++ + cb(counter === 2 ? false : null); + }, + cb => cb(null, true), + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }) + + it('doDuring canceling in test', (done) => { + let counter = 0; + async.doDuring( + cb => { + counter++ + cb(null, counter); + }, + (n, cb) => cb(n === 2 ? false : null, true), + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }) }); diff --git a/test/forever.js b/test/forever.js index e00a22d08..b0a75ec7c 100644 --- a/test/forever.js +++ b/test/forever.js @@ -38,5 +38,18 @@ describe('forever', function(){ done(); }); }); + + it('should cancel', (done) => { + var counter = 0; + async.forever(cb => { + counter++ + cb(counter === 2 ? false : null) + }, () => { throw new Error('should not get here') }) + + setTimeout(() => { + expect(counter).to.eql(2) + done() + }) + }) }); }); diff --git a/test/until.js b/test/until.js index e98b18403..2b82fe165 100644 --- a/test/until.js +++ b/test/until.js @@ -34,6 +34,22 @@ describe('until', function(){ ); }); + it('until canceling', (done) => { + let counter = 0; + async.until( + () => false, + cb => { + counter++ + cb(counter === 2 ? false: null); + }, + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }) + it('doUntil', function(done) { var call_order = []; var count = 0; @@ -92,4 +108,20 @@ describe('until', function(){ } ); }); + + it('doUntil canceling', (done) => { + let counter = 0; + async.doUntil( + cb => { + counter++ + cb(counter === 2 ? false: null); + }, + () => false, + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }) }); diff --git a/test/whilst.js b/test/whilst.js index e04c2b72f..2ce2f23ff 100644 --- a/test/whilst.js +++ b/test/whilst.js @@ -48,6 +48,22 @@ describe('whilst', function(){ done(); }); + it('whilst canceling', function(done) { + var counter = 0; + async.whilst( + function () { return counter < 3; }, + function (cb) { + counter++; + cb(counter === 2 ? false : null); + }, + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }); + it('doWhilst', function(done) { var call_order = []; @@ -122,4 +138,20 @@ describe('whilst', function(){ } ); }); + + it('doWhilst canceling', (done) => { + let counter = 0; + async.doWhilst( + cb => { + counter++ + cb(counter === 2 ? false : null); + }, + () => true, + () => { throw new Error('should not get here')} + ); + setTimeout(() => { + expect(counter).to.equal(2); + done(); + }, 10) + }) }); From cccd88961138247a15d026ae51d36705aed3a6d2 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 10 Jun 2018 18:46:45 -0700 Subject: [PATCH 07/14] fix waterfall test. It WILL get there --- lib/waterfall.js | 6 +++++- test/waterfall.js | 4 +--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/waterfall.js b/lib/waterfall.js index 10db92657..807d2ac13 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -67,6 +67,7 @@ export default function(tasks, callback) { if (!Array.isArray(tasks)) return callback(new Error('First argument to waterfall must be an array of functions')); if (!tasks.length) return callback(); var taskIndex = 0; + var canceled = false function nextTask(args) { var task = wrapAsync(tasks[taskIndex++]); @@ -75,7 +76,10 @@ export default function(tasks, callback) { } function next(err/*, ...args*/) { - if (err === false) return // canceled + if (err === false || canceled) { + canceled = true + return + } if (err || taskIndex === tasks.length) { return callback.apply(null, arguments); } diff --git a/test/waterfall.js b/test/waterfall.js index 8892f041d..1501f0332 100644 --- a/test/waterfall.js +++ b/test/waterfall.js @@ -153,9 +153,7 @@ describe("waterfall", function () { function(arg1, arg2, callback){ setTimeout(callback, 15, null, arg1, arg2, 'three'); } - ], function () { - throw new Error('should not get here') - }); + ]); }); it('call in another context @nycinvalid @nodeonly', function(done) { From 93591bf173082d01fdc8dbbb55039bc49befec70 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 10 Jun 2018 18:56:07 -0700 Subject: [PATCH 08/14] docs --- intro.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/intro.md b/intro.md index 43d3c819d..fe27c652a 100644 --- a/intro.md +++ b/intro.md @@ -173,6 +173,40 @@ async.map([1, 2, 3], AsyncSquaringLibrary.square.bind(AsyncSquaringLibrary), fun }); ``` +### Subtle Memory Leaks + +There are cases where you might want to exit early from async flow, when calling an Async method insice another async function: + +```javascript +function myFunction (args, outerCallback) { + async.waterfall([ + //... + function (arg, next) { + if (someImportantCondition()) { + return outerCallback(null) + } + }, + function (arg, next) {/*...*/} + ], function done (err) { + //... + }) +} +``` + +Something happened in a waterfall where you want to skip the rest of the execution, so you call an outer callack. However, Async will still wait for that inner `next` callback to be called, leaving some closure scope allocated. + +As of version 3.0, you can call any Async callback with `false` as the `error` argument, and the rest of the execution of the Async method will be stopped or ignored. + +```javascript + function (arg, next) { + if (someImportantCondition()) { + outerCallback(null) + return next(false) // ← signal that you called an outer callback + } + }, +``` + + ## Download The source is available for download from From 9952cf7b32cd74cbc94b4c6113a872cf75cafdd8 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 1 Jul 2018 15:39:13 -0700 Subject: [PATCH 09/14] auto should not start other tasks once canceled --- lib/auto.js | 3 ++- test/auto.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/auto.js b/lib/auto.js index 0175fda7b..500620a69 100644 --- a/lib/auto.js +++ b/lib/auto.js @@ -157,7 +157,8 @@ export default function (tasks, concurrency, callback) { } function processQueue() { - if (readyTasks.length === 0 && runningTasks === 0 && !canceled) { + if (canceled) return + if (readyTasks.length === 0 && runningTasks === 0) { return callback(null, results); } while(readyTasks.length && runningTasks < concurrency) { diff --git a/test/auto.js b/test/auto.js index 48de743b0..3b561cbef 100644 --- a/test/auto.js +++ b/test/auto.js @@ -193,6 +193,38 @@ describe('auto', function () { }, 10); }); + it('does not start other tasks when it has been canceled', function(done) { + const call_order = [] + debugger + async.auto({ + task1: function(callback) { + call_order.push(1); + // defer calling task2, so task3 has time to stop execution + async.setImmediate(callback); + }, + task2: ['task1', function( /*results, callback*/ ) { + call_order.push(2); + throw new Error('task2 should not be called'); + }], + task3: function(callback) { + call_order.push(3); + callback(false); + }, + task4: ['task3', function( /*results, callback*/ ) { + call_order.push(4); + throw new Error('task4 should not be called'); + }] + }, + function() { + throw new Error('should not get here') + }); + + setTimeout(() => { + expect(call_order).to.eql([1, 3]) + done() + }, 25) + }); + it('auto no callback', function(done){ async.auto({ task1: function(callback){callback();}, From c063c6b1e35447d7310b09cabec110ed02e9a6b6 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 1 Jul 2018 16:01:38 -0700 Subject: [PATCH 10/14] simplify waterfall, add test for arrays --- lib/waterfall.js | 5 +---- test/eachOf.js | 22 ++++++++++++++++++++++ test/forever.js | 2 +- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/waterfall.js b/lib/waterfall.js index 807d2ac13..0bd5dd736 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -76,10 +76,7 @@ export default function(tasks, callback) { } function next(err/*, ...args*/) { - if (err === false || canceled) { - canceled = true - return - } + if (err === false) return if (err || taskIndex === tasks.length) { return callback.apply(null, arguments); } diff --git a/test/eachOf.js b/test/eachOf.js index 791413a4a..0cd5e607b 100644 --- a/test/eachOf.js +++ b/test/eachOf.js @@ -444,6 +444,28 @@ describe("eachOf", function() { }, 20); }); + + it('forEachOfLimit canceled (async, array)', function(done) { + var obj = ['a', 'b', 'c', 'd', 'e']; + var call_order = []; + + async.eachOfLimit(obj, 3, function(value, key, callback){ + call_order.push(key, value); + setTimeout(() => { + if (value === 'b') { + return callback(false); + } + callback() + }) + }, function(){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([ 0, "a", 1, "b", 2, "c", 3, "d" ]); + done() + }, 20); + }); + it('forEachOfLimit canceled (async, w/ error)', function(done) { var obj = { a: 1, b: 2, c: 3, d: 4, e: 5 }; var call_order = []; diff --git a/test/forever.js b/test/forever.js index b0a75ec7c..b69839959 100644 --- a/test/forever.js +++ b/test/forever.js @@ -49,7 +49,7 @@ describe('forever', function(){ setTimeout(() => { expect(counter).to.eql(2) done() - }) + }, 10) }) }); }); From a85a3cdc33112ca403cecacc61f86f351fffe8db Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 1 Jul 2018 16:04:30 -0700 Subject: [PATCH 11/14] simplify eachOf --- intro.md | 2 +- lib/internal/eachOfLimit.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/intro.md b/intro.md index fe27c652a..2470954d8 100644 --- a/intro.md +++ b/intro.md @@ -175,7 +175,7 @@ async.map([1, 2, 3], AsyncSquaringLibrary.square.bind(AsyncSquaringLibrary), fun ### Subtle Memory Leaks -There are cases where you might want to exit early from async flow, when calling an Async method insice another async function: +There are cases where you might want to exit early from async flow, when calling an Async method inside another async function: ```javascript function myFunction (args, outerCallback) { diff --git a/lib/internal/eachOfLimit.js b/lib/internal/eachOfLimit.js index 6ca34aeed..ae153b1a2 100644 --- a/lib/internal/eachOfLimit.js +++ b/lib/internal/eachOfLimit.js @@ -19,8 +19,9 @@ export default function _eachOfLimit(limit) { var looping = false; function iterateeCallback(err, value) { + if (canceled) return running -= 1; - if (err && !canceled) { + if (err) { done = true; callback(err); } @@ -30,7 +31,6 @@ export default function _eachOfLimit(limit) { } else if (value === breakLoop || (done && running <= 0)) { done = true; - if (canceled) return return callback(null); } else if (!looping) { @@ -44,7 +44,7 @@ export default function _eachOfLimit(limit) { var elem = nextElem(); if (elem === null) { done = true; - if (running <= 0 && !canceled) { + if (running <= 0) { callback(null); } return; From ca547a5b15c6c1d3c13be8dba8a08632a23a61f2 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 1 Jul 2018 16:13:59 -0700 Subject: [PATCH 12/14] cancelable retry --- lib/retry.js | 1 + lib/waterfall.js | 1 - test/retry.js | 14 +++++++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/retry.js b/lib/retry.js index 390fd55cc..ef0440667 100644 --- a/lib/retry.js +++ b/lib/retry.js @@ -133,6 +133,7 @@ export default function retry(opts, task, callback) { var attempt = 1; function retryAttempt() { _task(function(err) { + if (err === false) return if (err && attempt++ < options.times && (typeof options.errorFilter != 'function' || options.errorFilter(err))) { diff --git a/lib/waterfall.js b/lib/waterfall.js index 0bd5dd736..ceaf59d87 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -67,7 +67,6 @@ export default function(tasks, callback) { if (!Array.isArray(tasks)) return callback(new Error('First argument to waterfall must be an array of functions')); if (!tasks.length) return callback(); var taskIndex = 0; - var canceled = false function nextTask(args) { var task = wrapAsync(tasks[taskIndex++]); diff --git a/test/retry.js b/test/retry.js index 8a9f4dacf..cbe240919 100644 --- a/test/retry.js +++ b/test/retry.js @@ -119,7 +119,19 @@ describe("retry", function () { setTimeout(function () { expect(calls).to.equal(5); done(); - }, 50); + }, 10); + }); + + it("should be cancelable", function (done) { + var calls = 0; + async.retry(2, function(cb) { + calls++; + cb(calls > 1 ? false : 'fail'); + }, () => { throw new Error('should not get here') }); + setTimeout(function () { + expect(calls).to.equal(2); + done(); + }, 10); }); it('retry does not precompute the intervals (#1226)', function(done) { From 69a27d6b001b63a14b37a2e33716bb9be714baae Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 1 Jul 2018 16:21:20 -0700 Subject: [PATCH 13/14] cancelable eachOf for arrays --- lib/eachOf.js | 7 ++++++- test/eachOf.js | 24 ++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/eachOf.js b/lib/eachOf.js index bb9edfac7..5dbeb81c2 100644 --- a/lib/eachOf.js +++ b/lib/eachOf.js @@ -12,12 +12,17 @@ function eachOfArrayLike(coll, iteratee, callback) { callback = once(callback || noop); var index = 0, completed = 0, - length = coll.length; + length = coll.length, + canceled = false; if (length === 0) { callback(null); } function iteratorCallback(err, value) { + if (err === false) { + canceled = true + } + if (canceled === true) return if (err) { callback(err); } else if ((++completed === length) || value === breakLoop) { diff --git a/test/eachOf.js b/test/eachOf.js index 0cd5e607b..7d80990c4 100644 --- a/test/eachOf.js +++ b/test/eachOf.js @@ -444,8 +444,7 @@ describe("eachOf", function() { }, 20); }); - - it('forEachOfLimit canceled (async, array)', function(done) { + it('eachOfLimit canceled (async, array)', function(done) { var obj = ['a', 'b', 'c', 'd', 'e']; var call_order = []; @@ -466,6 +465,27 @@ describe("eachOf", function() { }, 20); }); + it('eachOf canceled (async, array)', function(done) { + var arr = ['a', 'b', 'c', 'd', 'e']; + var call_order = []; + + async.eachOf(arr, function(value, key, callback){ + call_order.push(key, value); + setTimeout(() => { + if (value === 'b') { + return callback(false); + } + callback() + }) + }, function(){ + throw new Error('should not get here') + }); + setTimeout(() => { + expect(call_order).to.eql([ 0, "a", 1, "b", 2, "c", 3, "d", 4, "e" ]); + done() + }, 20); + }); + it('forEachOfLimit canceled (async, w/ error)', function(done) { var obj = { a: 1, b: 2, c: 3, d: 4, e: 5 }; var call_order = []; From 4382365580bd2aa4f0a817bf58a1cda949dabb14 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 1 Jul 2018 16:51:49 -0700 Subject: [PATCH 14/14] revert test tweak --- test/retry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/retry.js b/test/retry.js index cbe240919..d5e0d4052 100644 --- a/test/retry.js +++ b/test/retry.js @@ -119,7 +119,7 @@ describe("retry", function () { setTimeout(function () { expect(calls).to.equal(5); done(); - }, 10); + }, 50); }); it("should be cancelable", function (done) {