any() and some() behavior is nondeterministic and inconsistent #60

Closed
johnyanarella opened this Issue Oct 26, 2012 · 18 comments

Projects

None yet

3 participants

As currently implemented, given the same theoretical set of Promises, a call to any() or some() will produce a different result depending on the timing of when those Promises are resolved or rejected.

For example:

If a Promise in the supplied set is resolved before any of the other Promises are rejected, any() will resolve the Promise it returned with the resolved value. If one of the Promises in the supplied set is subsequently rejected, it is ignored because the returned Promise has already been resolved.

If the timing is slightly different, and a Promise in the supplied set is rejected before any of the Promises are resolved, any() will reject the Promise it returned with the rejected value. If one of the Promsies in the supplied set is subsequently resolved, it is ignored because the returned Promise has already been rejected.

any() is documented as:

Return a promise that will resolve when any one of the supplied promisesOrValues has resolved.

and some() is documented as:

Return a promise that will resolve when howMany of the supplied promisesOrValues have resolved.

so this behavior would appear to contradict the documentation.

In their current form, this nondeterministic behavior sets any() and some() apart from their all(), map(), and reduce() brethren. Evaluating the same theoretical set of Promises, all(), map() and reduce() will always produce the same result, regardless of the timing of their resolution or rejection.

I noticed earlier this week that someone else reached out to @briancavalier on Twitter to demonstrate that passing an Array containing a rejected Promise and a resolved Promise to any() produces a Promise that rejects instead of resolving. This reveals that any() and some() in their current form can also potentially give greater weight to Promises that appear earlier in the supplied Array.

I believe that the correct behavior would be for any() and some() to defer rejection until after all potential resolutions have been ruled out. So, any() would resolve as soon as a Promise resolves but wouldn't reject until all of the supplied Promises have been rejected (and there is no possibility of a Promise resolving). Similarly, some() would resolve as soon as the specified number of Promises resolve, and would only reject once there was an insufficient number of pending Promises available to meet that threshold.

This also raises the question of what rejection value should be returned. Currently, the first rejection value is returned. This also seems odd (and the same applies to all(), map(), and reduce()).

I'm starting to think that all of these utility methods should return consistent rejection values that are oriented relative to the attempt to perform that operation.

If a developer wants to be aware of the reason why a specific Promise in the supplied set was rejected or wants to add recovery logic, that should be applied to the originating Promise (and that recoverable Promise should be supplied in the set instead) rather than to the aggregate utility operation.

If they are adding fault handling or recovery logic to the Promise returned by the aggregate utility method, that should be considered as being relative to the failure of that method's operation, not the individual elements originally supplied to that method.

Thoughts?

Owner

To be completely honest, I've felt for while that the current implementations of any() and some() are wonky, and possibly just flat out wrong. Unfortunately, it just hasn't been a priority for me to address them yet. There are comments in the code about possible better approaches, and I'll be revisiting for v2.0.

They will always be (or at least have the chance of being) nondeterministic: If the promises involved are truly async (e.g. XHR) then it'll always be a race for which one(s) will finish first.

I agree that handling rejections probably needs to be improved for any() and some(). I'm not (yet) convinced of that with respect to all(), map(), and reduce(). Since the whole operation fails when any one of the input promises rejects, it makes sense to me to allow that rejection to propagate out--although wrapping it in some way may be an option.

If any/some() were to fail only when say (length - howMany) + 1 input promises rejected, then that will make them very different beasts than all/map/reduce, and some richer rejection value will probably be necessary.

mkuklis commented Oct 26, 2012

I ran into the same issues described by @johnyanarella (very well written btw). I ran into situation where I had to run 2 separate queries against DB. The time taken to execute each query and get the results was different each time. Which means first query could execute before the second or vice versa. It was sufficient to get the successful result from only one query in order to continue. That's the reason I've decided to use any / some.

I realized however that if the query which finishes first doesn't return any results and the call is rejected with deferred.reject the any just fails and the operation never proceeds.

I think what I expected from any was to proceed as soon as the first deferred.resolve is called but don't reject the whole operation if deferred.reject is called first. In other words it with would be nice to just continue until the first deferred.resolve is found and reject the whole operation only after all deferred were rejected.

The example I've created before which illustrates this: http://jsfiddle.net/mkuklis/ektG9/3/

Thanks for the (as always) thoughtful response, @briancavalier.

There are two degrees of nondeterministicness (nondeterministicosity? nondeterministicitude?) at play here. Given the way these methods are named and documented, I would hope that most developers would anticipate the nondeterministic nature as to which specific resolution value(s) "win" in an asynchronous race. In contrast, I think most developers will be taken off guard when the overall success or failure of the operation itself is nondeterministic for the same set of inputs.

Most use cases that I would imagine for any() and some() relate to using the first available resolved Promise(s) among a set of Promises where some may reject.

Owner

Thanks for the use case, @mkuklis. Having a real use case helps a lot. Recently, I've started thinking of any/some as a way to initiate competitive races, and from what I can tell, that line of thought matches your use case.

Like you guys said, right now, any/some reject immediately on the first input promise rejection that occurs, if that rejection occurs before the desired number of input promises have resolved. Given the "competitive race" mental model, I think the right behavior is: reject when it becomes impossible to fulfill what the caller asked for. IOW, when (length - howMany) + 1 inputs have rejected, it is simply impossible to resolve howMany of the inputs.

Thoughts on the "competitive race" mental model?

Owner

@johnyanarella, just to make sure my understanding of the two degrees of nondeterminism match yours :). The two I see are:

  1. Nondeterministic wrt whether the entire operation will resolve or reject at all, simply based on "arrival time" of the first rejection, even if the overall operation still would have a chance of succeeding (i.e. howMany inputs might have still resolved if they had won the race against the one rejected input)
  2. In the reject case, nondeterministic wrt which one rejected input causes the entire operation to reject.
  3. Nondeterministic in the resolution case wrt which resolutions (up to howMany) win.

Ok, ok, that's 3 :) The first two definitely seem like problems, and I think can be "fixed". The third is the whole point of a competitive race, and so would (intentionally) remain.

Does that make sense? Does it match your view of what developers might expect?

Owner

Oops, I said ""arrival time" of the first resolution" above, when I meant ""arrival time" of the first rejection". Edited/Corrected

Owner

FWIW, this paragraph from @johnyanarella's (excellent) initial description also seems to point to a competitive race model:

I believe that the correct behavior would be for any() and some() to defer rejection until after all potential resolutions have been ruled out. So, any() would resolve as soon as a Promise resolves but wouldn't reject until all of the supplied Promises have been rejected (and there is no possibility of a Promise resolving). Similarly, some() would resolve as soon as the specified number of Promises resolve, and would only reject once there was an insufficient number of pending Promises available to meet that threshold.

Makes total sense and matches my expectations as a developer.

Framing it as a "competitive race" captures the idea perfectly. Brilliant.

mkuklis commented Oct 26, 2012

This makes sense to me too. Thanks!

@briancavalier briancavalier added a commit that referenced this issue Oct 26, 2012
@briancavalier briancavalier See #60. Refactor some() to initiate a competitive race that only rej…
…ects once it becomes impossible to resolve howMany inputs
da394fa
Owner

This just landed in the dev branch if you want to try it out.

mkuklis commented Oct 28, 2012

Great! Giving it a try now!

mkuklis commented Oct 28, 2012

Just wanted to report this works as expected now :) Thank you! Here is a snippet from my actual code:

/**
 * Finds user by email or username.
 *
 * @param {String} query
 */
User.findByEmailOrUsername = function (query) {
  var promises = [];
  promises.push(User.exists({ email: query }));
  promises.push(User.exists({ username: query }));

  return when.any(promises);
}

/**
 * Tests if user exists based on given parameters.
 *
 * returns user's id if user exists and false otherwise.
 * @param {Object} search
 */
User.exists = function (search) {
  var deferred = when.defer();

  User.find(search, function (err, ids) {
    if (err || ids.length == 0) {
      deferred.reject(new Error('user not found'));
    }
    else {
      deferred.resolve(ids[0]);
    }
  });

  return deferred.promise;
}
Owner

Great, thanks for testing and posting a followup! That's a nice little code snippet. Are you open to contributing it as an example of how to use any()? I'm thinking we could add it to the wiki and link to it from the API docs.

mkuklis commented Oct 28, 2012

Sure please go ahead and add it to the wiki if you think it's valuable.

Owner

Thanks! I'll add it asap.

FYI, just released when.js 1.6.0 which includes this stuff :)

mkuklis commented Oct 31, 2012

Great thanks! Will switch to 1.6.0

Owner

Added that example to the wiki. I'll write some text around it soon.

mkuklis commented Nov 5, 2012

great :) thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment