cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Unexpected behaviour of when.map and when.guard.n #440

Closed ChrisEineke closed 9 years ago

ChrisEineke commented 9 years ago

Unlike Array.prototype.map, which iterates over each element one by one and in sequence, when.map will immediately create a promise for each value in the array. If one promise rejects, the promise returned by when.map will reject, but all other in-flight promises will continue to settle.

While one can limit concurrency via guard.n(1), it doesn't get rid of the remaining in-flight promises as guard.n() still queues up execution for each promise.

I suggest that when.map should behave like Array.prototype.map, in that it should iterate one by one and in sequence. The existing function can then be renamed to when.pmap that exhibits the old behaviour and can be limited by guard.n(1) as usual.

ChrisEineke commented 9 years ago

To give a real-world example, I encountered this while using promised-temp to create a temp directory and tried to download a bunch of files, but only one at a time. I wrapped my downloader in a guard.n(1) and applied it to an array of filenames using when.map. Whenever a file failed to download (and that file wasn't the last file to be downloaded), other files after the failed one were still attempted to be downloaded but eventually lead to an uncaughtException because the temp directory did no longer exist.

rkaw92 commented 9 years ago

Dnia 23 marca 2015 03:42:21 CET, Christopher Eineke notifications@github.com napisał(a):

To give a real-world example, I encountered this while using promised-temp to create a temp directory and tried to download a bunch of files, but only one at a time. I wrapped my downloader in a guard.n(1) and applied it to an array of filenames using when.map. Whenever a file failed to download (and that file wasn't the last file to be downloaded), other files after the failed one were still attempted to be downloaded but eventually lead to an uncaughtException because the temp directory did no longer exist.


Reply to this email directly or view it on GitHub: https://github.com/cujojs/when/issues/440#issuecomment-84761582

Hi,

I think you are trying to do conditional task execution based on when.all(). Promises do not do that, because they represent results of operations, not the operations themselves.

That is, it is your task runner (when.parallel?) that needs to guarantee this cancellation of subsequent operations. Settling promises has nothing to do with it - the task still executes, even if there are no callbacks for its promise's resolution. In other words, the falling tree does make a sound, even in the absence of any observers.

Out of curiosity, why are you not running the tasks in a sequence? That way, you get a causality guarantee (if task A fails and triggers and early exit, and B is after A, task B will not execute).

Another solution would be to await the success of all the downloads (when.all), but also register a callback on the resolution of all promises via when.settle. You then obtain two derived promises - one's resolution semantics being suited for reacting to success, the other's for handling clean-up tasks.

briancavalier commented 9 years ago

@ceineke Good questions, and @rkaw92 has, as usual :D, provided good answers and suggestions.

I'll add my own $.02.

First, guard is an operation that limits concurrency, rather than guaranteeing causality.

Second, mathematically speaking, "map" is an abstract operation that can be applied to many data structures: lists, trees, queues, and even sets. If you consider a set, whose items, by definition, have no order, it is clear that "map" is an atomic operation that either fails or succeeds as a whole. The fact that Array.prototype.map always proceeds from lower indices to higher indices, is an implementation detail stemming from the fact that the simplest and most obvious way to map an Array in JavaScript is to use a loop that goes from 0 to < array.length. It would be equally correct to loop from array.length-1 to >= 0, or even in random order.

The combination of guard and when.map limits the concurrency by which it may make progress towards its atomic goal, and when.map will fail or succeed atomically. It does not make ordering guarantees.

Unfortunately, it is reality that promises have no agreed-upon cancelation semantics right now. It has been discussed at great length in the community and progress has been made, but nothing's been finalized yet. Thus, at the moment, "canceling" concurrent inflight when.map tasks is something that must be implemented by the caller.

All of that said, when.reduce (and Array.prototype.reduce) provides causal ordering. You can easily implement a causal (i.e. strictly ordered) map operation by reducing one array and building up another:

function mapSequence(promises, f) {
  return when.reduce(promises, function(result, x) {
    return when(x, f).then(function(y) {
      result.push(y);
      return result;
    });
  }, []);
}

tried to download a bunch of files, but only one at a time

The best choices for modeling this behavior are when.reduce, when/sequence, and when/pipeline, which all solve the one at a time with causal ordering scenario, whereas when.map isn't intended to solve that.

Have a look at those, and let me know if you have questions about using them for your situation. Hope that helps!

ChrisEineke commented 9 years ago

Thanks for the explanation, Brian. It might be worthwhile to update the documentation to explain the differences in behaviour between the synchronous Array.prototype.map and the asynchronous when.map.

briancavalier commented 9 years ago

update the documentation to explain the differences in behaviour between the synchronous Array.prototype.map and the asynchronous when.map

Seems like a great idea! I'll open a new ticket for that.

Shall we close this one?

ChrisEineke commented 9 years ago

Please.