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.

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

auto allows functions to start after error is passed #988

Closed
tekwiz opened this issue Dec 23, 2015 · 3 comments · Fixed by #993
Closed

auto allows functions to start after error is passed #988

tekwiz opened this issue Dec 23, 2015 · 3 comments · Fixed by #993

Comments

@tekwiz
Copy link
Contributor

tekwiz commented Dec 23, 2015

After a task in an async.auto calls back with an error (in the following example, this is task x) it should stop the processing of future tasks. However, it still allows the task c to run.

Example:

async.auto({
  a: function(next) {
    console.time('a:Time');
    console.log('a()');
    setTimeout(function() {
      next(null, 'one');
      console.timeEnd('a:Time');
    }, 200);
  },
  b: function(next) {
    console.time('b:Time');
    console.log('b()');
    setTimeout(function() {
      next(null, 'two');
      console.timeEnd('b:Time');
    }, 50);
  },
  c: ['a', 'b', function(next) {
    console.time('c:Time');
    console.log('c()');
    next(null, { three: 3 });
    console.timeEnd('c:Time');
  }],
  x: function(next) {
    console.time('x:Time');
    console.log('x()');
    setTimeout(function() {
      next(new Error('x'));
      console.timeEnd('x:Time');
    }, 100);
  }
}, function(err, results) {
  console.log('callback(%s, %j)', err, results);
});

Output:

a()
b()
x()
b:Time: 56ms
callback(Error: x, {"b":"two"})
x:Time: 101ms
a:Time: 205ms
c()
c:Time: 0ms

A quick explanation of what is happening (and what should be happening): a, b, and x immediately begin executing (since they have no dependencies). b completes first and x calls-back with an error. At this point the final callback properly executes followed by x and a. The following execution of c is the failure because the error from x is called-back before a (the dependency of c) can complete.

That said, I can see a valid argument for this functionality: i.e. because c does not depend on x it should be allowed to execute; however, because the callback runs immediately upon receiving an error, can only handle a single error, and does not include the results from c it seems that the auto function should dump any remaining tasks.

@gr2m
Copy link
Contributor

gr2m commented Dec 30, 2015

I run into the same problem, here is a simpler test case, and it sets concurrencty to 1, so I’d say it definitely is bug, as it continues after a task return an error:

var assert = require('assert')
var async = require('async')

async.auto({
  task1: function (callback) {
    callback('error')
  },
  task2: function (callback) {
    assert.fail('task2 should not be called')
  }
}, 1, function (error) {
  assert.equal(error, 'error', 'callback with "error"')
})

you can see it run here: https://tonicdev.com/gr2m/5683f3895728b70d0082625d

@gr2m
Copy link
Contributor

gr2m commented Dec 30, 2015

I’ll look into this and send a PR today

gr2m added a commit to gr2m/async that referenced this issue Dec 30, 2015
@gr2m gr2m mentioned this issue Dec 30, 2015
2 tasks
gr2m added a commit to gr2m/async that referenced this issue Dec 30, 2015
@gr2m
Copy link
Contributor

gr2m commented Dec 30, 2015

My PR is ready: #993

@tekwiz this won’t fix your problem, because the tasks are run in parallel by design as you mentiond, but if you add the 1 for concurrency option, the output is

a()
a:Time: 206ms
x()
callback(Error: x, {"a":"one"})
x:Time: 104ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants