"Reflect" behavior for collecting errors/results and continuing.

[aearly]

Related to #349 #334 #531 .

There have been many requests for a behavior where when an iterator calls back with an error, the error is collected, and execution continues. There have been a couple attempts at this feature, but the implementations fall short.

In theory, this would be applied to just about any async function, map, series, parallel, forEachSeries, etc.. I'd like to support this broadly, but without having to create duplicate implementations for each function. Something we can wrap existing functions with, without having to modify the implementations.

We also need to settle on what the name is for this type of behavior. These have been proposed:

• mapCollect / parallelCollect / eachSeriesCollect
• mapContinue / parallelContinue / eachSeriesContinue
• mapAnyway / parallelAnyway / eachSeriesAnyway
• mapMaybe / parallelMaybe / eachSeriesMaybe
• mapResume / parallelResume / eachSeriesResume

Lastly -- how should the collected results be passed to the final callback?

• callback(null, {errors: [], results: []])
• callback(null, [Error, results, results, Error, results..])
• callback(null, [{err: null, result: results0}, {err: Error, result: null}, ...])
• callback(null, results, errors)

[megawac]

I would suggest callback(null, results, errors) or callback(null, [Error, obj, obj, Error, etc..])

[tommy31]

Any update of this?

[aearly]

Sorry, we've been too busy to make any progress on this. We also haven't decided on the naming pattern, and haven't thought up a clean way to add it to the library.

[jtwebman]

Actually bluebird is switching to a map with a reflect wrapper around each. Maybe that would be easier? So reflect would wrap each of the callbacks and always return successful. The output would then have null as error and a list of reflect objects that with have a function hasValue() method that returns true if the reflect .value property has a value (ie. didn't error) and if it did error then the .error property will have the error.

Here is what the code would look like.

async.series([
  async.reflect(function(callback){
    // do some stuff ...
    callback(null, 'one');
  }),
  async.reflect(function(callback){
    // do some more stuff but error ...
    callback(new Error('bad stuff happened'));
  }),
  async.reflect(function(callback){
    // do some more stuff ...
    callback(null, 'two');
  })
],
// optional callback
function(err, results){
  // values
  // results[0].value = 'one'
  // results[1].error = Error('bad stuff happened')
  // results[2].value = 'two'
});

[jtwebman]

We can add a reflectAll function as well that can take a list of functions with callbacks and wrap them all in async.reflect like this:

let tasks = [
  function(callback){
    setTimeout(function(){
        callback(null, 'one');
    }, 200);
  },
  function(callback){
    // do some more stuff but error ...
    callback(new Error('bad stuff happened'));
  }
  function(callback){
    setTimeout(function(){
      callback(null, 'two');
    }, 100);
  }
];

async.parallel(async.reflectAll(tasks),
// optional callback
function(err, results){
  // values
  // results[0].value = 'one'
  // results[1].error = Error('bad stuff happened')
  // results[2].value = 'two'
});

[aearly]

@jtwebman I like your approach. It means we don't have to create new methods for each flavor of async function, leaving it to the user to use it when continuing is desired. Also, +1 for the consistency with Bluebird's direction.

[danielshir]

Great idea, although the only thing that irks me is wrapping the tasks themselves.
The code already has major boilerplate that it requires, so how about using a sub object?
I write most of my code inline (e.g. not using a "tasks" var) so I'd prefer it this way.

Say something like :

let tasks = [
  function(callback){
    setTimeout(function(){
        callback(null, 'one');
    }, 200);
  },
  function(callback){
    // do some more stuff but error ...
    callback(new Error('bad stuff happened'));
  }
  function(callback){
    setTimeout(function(){
      callback(null, 'two');
    }, 100);
  }
];

async.reflect().parallel(tasks),
// optional callback
function(err, results){
  // values
  // results[0].value = 'one'
  // results[1].error = Error('bad stuff happened')
  // results[2].value = 'two'
});

[jtwebman]

@danielshir How is that different then this:

let tasks = [
  function(callback){
    setTimeout(function(){
        callback(null, 'one');
    }, 200);
  },
  function(callback){
    // do some more stuff but error ...
    callback(new Error('bad stuff happened'));
  }
  function(callback){
    setTimeout(function(){
      callback(null, 'two');
    }, 100);
  }
];

async.parallel(async.reflectAll(tasks),
// optional callback
function(err, results){
  // values
  // results[0].value = 'one'
  // results[1].error = Error('bad stuff happened')
  // results[2].value = 'two'
});

This way is more explicit without having to hide some magic under some object that treats parallel or any other method different plus not all methods make sense to use reflect.

[danielshir]

Well, if that's your writing style then I guess it looks the same.
But, what if you write your functions inline? As I assume most people do.

It's a bit messy for my taste since you're adding another scope (more parentheses) -

async.parallel(async.reflectAll([function(callback){
    setTimeout(function(){
        callback(null, 'one');
    }, 200);
  },
  function(callback){
    // do some more stuff but error ...
    callback(new Error('bad stuff happened'));
  }
  function(callback){
    setTimeout(function(){
      callback(null, 'two');
    }, 100);
}]
),
// optional callback
function(err, results){
  // values
  // results[0].value = 'one'
  // results[1].error = Error('bad stuff happened')
  // results[2].value = 'two'
});

[jtwebman]

@danielshir Then you can write it like this if you want:

async.parallel([function(callback){
    setTimeout(function(){
        callback(null, 'one');
    }, 200);
  },
  function(callback){
    // do some more stuff but error ...
    callback(new Error('bad stuff happened'));
  }
  function(callback){
    setTimeout(function(){
      callback(null, 'two');
    }, 100);
}].map(async.reflect),
// optional callback
function(err, results){
  // values
  // results[0].value = 'one'
  // results[1].error = Error('bad stuff happened')
  // results[2].value = 'two'
});

Leaving these just functions keeps it very simple and doesn't spread the code to do this all over the place.

[danielshir]

@jtwebman I didn't think of that and that actually looks way better!
The reflect call is right near the finishing callback which will remind the programmer why results look like they do 👍

[enrichz]

I'm very interested in this feature, thanks for your work guys. +1

[mahlu]

+1

[akayami]

Great feature for me, but I do not see this in 2.0.0-rc.1 - Any hope it will be ready soon ?

[aearly]

We're waiting for #1012 to be completed. No ETA. It might wait until 2.1.

[aearly]

Closed via #1095 !