-
Notifications
You must be signed in to change notification settings - Fork 396
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
Unexpected behaviour of when.map and when.guard.n #440
Comments
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. |
Dnia 23 marca 2015 03:42:21 CET, Christopher Eineke notifications@github.com napisał(a):
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. |
@ceineke Good questions, and @rkaw92 has, as usual :D, provided good answers and suggestions. I'll add my own $.02. First, 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 The combination of 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 All of that said, function mapSequence(promises, f) {
return when.reduce(promises, function(result, x) {
return when(x, f).then(function(y) {
result.push(y);
return result;
});
}, []);
}
The best choices for modeling this behavior are Have a look at those, and let me know if you have questions about using them for your situation. Hope that helps! |
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. |
Seems like a great idea! I'll open a new ticket for that. Shall we close this one? |
Please. |
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.
The text was updated successfully, but these errors were encountered: