Unspecified behavior when onFulfilled or onRejected throws a promise object

[lsmith]

Per my note on issue 7 of the resolvers spec, the spec language for 3.2.6.2 says

If either onFulfilled or onRejected throws an exception, promise2 must be rejected with the thrown exception as the reason.

But "exception" isn't defined, so

promise.then(function () {
    throw otherPromise;
}).then(what?);

is possible.

It's edge case city, but it is a loophole that could be considered a legal way of passing a promise as a value between callbacks (besides return [otherPromise]). Otherwise, it's an opportunity for implementation iniquity, and my guess would be that Unexpected Things are bound to happen in the pass-through mechanism when subsequent onRejected callbacks aren't supplied.

[briancavalier]

Good catch. IMHO, this should always log 'REJECTED':

promise.then(function () {
    throw anyPendingFulfilledOrRejectedPromiseFor('REJECTED');
}).then(
    function() { console.log('FULFILLED'); },
    function(e) { console.log(e); }
);

But I just checked and when.js simply hands the otherPromise to the rejection handler, which seems totally wrong.

[lsmith]

This should also get coverage in the test suite, once we decide what should happen :)

[briancavalier]

@lsmith 👍

[domenic]

This actually is in the test suite, where exception is taken to mean the same thing it means in the ES5 spec: any value (including a promise).

Thus I think that indeed otherPromise should be handled to the rejection handler.

[domenic]

I do think clarifying the definition of exception would be valuable, however. It must by design be the same definition as "reason", which is itself the same as "value", so my point stands.

[briancavalier]

@domenic I disagree for the same reasons that I don't like fulfill(). IMHO, it should be impossible to observe the difference between these two:

promise.then(function() { throw 123; });
// and
promise.then(function() { throw createAPromiseFor(123); });

Just as it is impossible to observe the difference between:

promise.then(function() { return 123; });
// and
promise.then(function() { return createFulfilledPromise(123); });

Otherwise, we may break the identity function again:

function success(x) { console.log('SUCCESS', x); }
function fail(x) { console.error('FAIL', x); }

// Logs SUCCESS 123
promise.then(function() { throw 123; })
    .then(null, identity).then(success, fail);

// Logs FAIL 123
// The identity function should recover from a failure here because
// it does not explicitly propagate the rejection.  However, it *unknowingly*
// (and probably mistakenly) propagates the rejection simply by returning its input
promise.then(function() { throw createRejectedPromise(123); })
    .then(null, identity).then(success, fail);

[domenic]

OK, after putting that on hold for a few days before reading it, that's a pretty compelling argument.

@kriskowal, @wycats, @ForbesLindesay, @novemberborn, any objections? Given that Q and every other promise library seems to mistakenly propagate the rejection, we should make sure before we make this change.

This, combined with #58, seem to me enough for a 1.1. If there are no objections by, say, Saturday, I'll give a go at making the changes, plus updating the website with new pages like revision history and list of conformant implementations.

[domenic]

Wait. It's bugging me that throw fulfilledPromise rejects. Some symmetry is lost:

let f be a fulfilled promise with fulfillment value F, r a rejected one with reason R, and V a non-promise value. Then, under your plan in promises-aplus/constructor-spec#10:

resolve(f) → fulfilled, F
return f → fulfilled, F
throw f → rejected, F

resolve(r) → rejected, R
return r → rejected, R
throw r → rejected, R

resolve(V) → fulfilled, V
return V → fulfilled, V
throw V → rejected, V

It seems like throw f should give fulfilled, F, as to me a fulfilled promise is more like a rejected promise than it is like a value. Convince me otherwise?

I guess in my mind, throw/return when applied to promises both signal resolve behavior, instead of return signaling resolve behavior and throw signaling some kind of transform-to-rejection behavior.

[kriskowal]

This is needless fretting. Throwing a promise is outside the defined behavior of a promise system.

[ForbesLindesay]

@kriskowal I agree, we need to be careful not to make a spec that's needlessly difficult for new libraries to adhere to if it's an edge case that doesn't cause real compatibility issues.

@domenic I think throw needs to always become a rejection, anything else is bizzar. I generally view return as a shortcut for nextResolver.resolve(...) and throw as a shortcut for nextResolver.reject(...) I'd say that rejecting with a resolved promise should either pass that promise on as the rejection reason, or pass the result of that promise on as a rejection reason. I'm not sure which, and I'm not that convinced that it matters provided we all do the same.

[novemberborn]

I'm +1 with @ForbesLindesay.

[briancavalier]

While I agree with @kriskowal that throwing a promise is basically nonsensical, Javascript is not inherently a promise system, even though in practice, it is a highly asynchronous system. Practically speaking, for better or for worse, devs mix sync and async all the time. They'll throw whatever they want whenever they want, and they'll use sync libraries that do the same. So, I think we need to specify this for consistency.

I share @ForbesLindesay's mental model that return == resolve and throw == reject. I also feel strongly that we should never "leak" a promise directly to an onFulfilled or onRejected handler, for the reasons I gave above. Putting those two things together, the only thing that makes sense to me is for throw promise to reject with the resolution (fulfill or reject) value of promise.

[novemberborn]

Putting those two things together, the only thing that makes sense to me is for throw promise to reject with the resolution (fulfill or reject) value of promise.

Which is close enough to how return promise behaves, with the exception that that will reject if promise is rejected, and this will always reject.

+1.

[domenic]

@briancavalier

I share @ForbesLindesay's mental model that return == resolve and throw == reject. I also feel strongly that we should never "leak" a promise directly to an onFulfilled or onRejected handler, for the reasons I gave above.

Although I'm willing to get behind your proposed semantics for throw, I don't think I can support this line of reasoning fully. In particular, you cannot put resolve and reject on the same plane. It is fulfill and reject that are similar, whereas resolve is a meta-operation. More in promises-aplus/constructor-spec#10.

[briancavalier]

@domenic I agree that there's some tension there, given that we've used "reject" and "fulfill" in the promises-spec in a way that makes them sound parallel. However, I don't think that a polymorphic reject API for resolvers conflicts with our usage of the word "reject". Ultimately, it will always result in a rejected promise, whereas resolve was problematic because it might result in either a fulfilled or rejected promise.

It seems like we're close to being ok with changing 3.2.6.2 to specify the additional cases for throwing a promise. Are there any other objections or concerns?

[lsmith]

This is a tough one.

1. If reject waits for promises, the analogy of a promise representing two logic flows is abandoned. Both fulfilled and rejected promises will reject the parent promise. It doesn't make sense for fulfilled thrown promises to fulfill the parent promise because that would contradict the throw = reject contract, and having rejected thrown promises fulfill the parent promise is like saying two wrongs make a right, IMO.
2. If reject passes the promise through as the rejection reason, there is a lack of symmetry in the API. Return does not allow promises to propagate through, but throw does. The generic resolve is mapped to return, but a no-magic reject method is mapped to throw without a no-magic method counterpart for fulfill.

If it's decided to wait for promises, I would suggest that throw rejectedPromise should reject the parent promise with null or undefined, not with rejectedPromise's reason. throw rejectedPromise translates for me to "something went wrong, and we'll tell you what in a moment--strike that, we won't tell you", not to "something went wrong, and we'll tell you what in a moment--well there was a problem getting that reason for you, but the problem getting the reason should be an acceptable replacement for the actual reason."

Given there doesn't seem to be a clean solution here, I'm thinking erring on the side of de facto is preferable, because we want existing implementations that agree on the important things to be considered compliant rather than introducing a spec that nobody satisfies. That amounts to a loose +1 for passing the promise as the reason. If implementation authors widely agree that handling thrown promises is something worth changing their code for, then I'm fine with that. The other option of stating in a spec note that the handling of thrown promises is unspecified is undesirable, IMO.

[briancavalier]

@lsmith Nice 2-bullet summary. The asymmetry between return and throw allowing promises through (bullet 2) def bothers me. I'm actually less concerned about finding the "exactly right" behavior (if it exists) in the case where someone throws a promise, and more about the fact that throw leaks promises to handlers.

Of course, making throw promise force a rejection also means some asymmetry between return and throw, in that return can lead to fulfillment, but throw cannot. I'm ok with that. It means the only path to fulfillment is by returning successfully, which seems reasonable.

I think I'd be ok with going with whatever seems simplest between rejecting with null/undefined, or with the "replacement" reason. My current thinking is that simply letting the replacement reason propagate would require no special casing, but maybe the code is simple for either. I'm also bristling slightly at squelching the later error, as I feel like it may be more helpful in tracking down a problem than null/undefined would be. Mostly that's speculation, though.

[ForbesLindesay]

As always I think we should look to sync code for what to do.

function return5() { return 5; }
function throw5() { throw 5; }

try {
  throw return5();
} catch (ex) {
  assert(ex ===5);
}

try {
  throw throw5();
} catch (ex) {
  assert(ex ===5);
}

[domenic]

@ForbesLindesay +1, that convinced me.

[lsmith]

Something is bothering me about the sync example. I don't think it's analogous, but I can't put my finger on where the discrepancy is.

Regardless, I'm not passionate about either side and this is super edge casey, so whatever you guys agree on is fine by me.

[ForbesLindesay]

@lsmith It should be analogues to

function return5() { return new FulfilledPromise(5); }
function throw5() { return new RejectedPromise(5); }

resolve()
  .then(function () {
    throw return5();
  })
  .catch(function (ex) {
    assert(ex ===5);
  });

resolve()
  .then(function () {
    throw throw5();
  })
  .catch(function (ex) {
    assert(ex ===5);
  });

[briancavalier]

@ForbesLindesay Very interesting. And for completeness:

// This one is successful
resolve()
  .then(function () {
    return return5();
  })
  .catch(function (ex) {
    assert(ex ===5);
  });

// This one is not
resolve()
  .then(function () {
    return throw5();
  })
  .catch(function (ex) {
    assert(ex ===5);
  });

So, the only path to a successful result is returning a fulfilled promise (or non-promise, which is basically the same thing).

[domenic]

I am suddenly curious how other languages' future/promise libraries handle this. Anyone want to do a test in their favorite language? I'll try to whip up C#, if I still remember how to do that.

[domenic]

Ah of course, C# only allows you to throw things derived from System.Exception. Nevermind...

[kriskowal]

@domenic As well we should pretend in JavaScript.

[briancavalier]

That's def a valid question, and it'd be good if we could get some data points if possible. I agree it'd be nice if JS allowed throwing only Error instances or descendants, so we could ignore this situation. I feel like we need to specify something since JS allows throwing anything.

[briancavalier]

I guess the question has to be asked: Should we even consider next-tick-throw-crashing in response to someone throwing a promise??

[ForbesLindesay]

I don't think so, I think we should match Synchronous JavaScript as much as possible. That means we should let people throw whatever they like. But in our libraries we should only ever throw real exceptions.

[briancavalier]

Anyone have any concerns before I merge #66?

[domenic]

I'd like to get buy-in from @kriskowal and @wycats.

[briancavalier]

Paging @kriskowal and @wycats for buy-in or concerns.

[kriskowal]

A promise is not a valid exception. I won’t bloat Q with code to handle the case.

[domenic]

Welp. Last try: @kriskowal, this is all about preventing promises-for-promises from being created, which I believe you in the past have expressed a desire to do. I guess your stance is that promises-for-promises are OK, if they're created via throw instead of fulfill?

[kriskowal]

The case of a pending promise becoming a pending promise is not the same as a pending promise being rejected with a promise for an exception. I am content to omit fulfill and mandate that exceptions must be exceptions. Both decisions make the implementation and usage simpler.

[ForbesLindesay]

@kriskowal when you say:

mandate that exceptions must be exceptions

Do you mean precluding "values" from being exceptions? I often see libraries that "throw" plain objects.

How do you intend to mandate? What should the behavior of the following be:

function thows() {
  return Q.resolve(null)
    .then(function () {
      throw Q.resolve(new Error('foo'));
    });
}

[domenic]

After talking about this with MarkM, I am convinced that our current specified behavior is fine. "Promises for promises" is a misnomer, because we are treating fulfillment and rejection too symmetrically. You can reject with any exception, even an exception that is a promise, and that's valid. Exceptions have no special semantics. You shouldn't be able to fulfill with a promise (and indeed, the base spec gives no method for doing this), since that's a true "promise for a promise," but rejecting with one is OK.

Besides, lack of consensus kills this idea dead in the water.

[briancavalier]

It may be dead due to lack of consensus, but I don't understand the reasoning behind some of the things you said:

"Promises for promises" is a misnomer, because we are treating fulfillment and rejection too symmetrically. You can reject with any exception, even an exception that is a promise, and that's valid.

Can you explain "too symmetrically" and why it is bad? Why is it valid to use promise-as-an-exception verbatim?

[domenic]

I wish I could remember exactly how MarkM phrased it (@kriskowal may recall better), but essentially: promises represent eventual and/or remote values. A rejected promise takes you out of that paradigm, representing an eventual/remote thrown exception. The fulfillment value is primary, especially if you think of the remote object case: it's what's really underlying the promise. The exception is just a reason that the primary use case of the promise couldn't be fulfilled. Because we are focused on the then method here, we treat fulfilled and rejected states with equal importance, but in reality fulfilled is the "natural" state of a promise. In particular the idea of waiting asynchronously on a rejection reason doesn't make as much sense as waiting asynchronously on a fulfillment value. Thus, using a promise as an exception is just as valid as using an integer or undefined: it's silly, but it's not a problem.

[briancavalier]

Thanks. This is very interesting, and maybe somewhat of a lightbulb moment for me. It seems like what MarkM is saying is that we can look at a promise as being either fulfilled or not-fulfilled, which is perhaps subtly different from fulfilled, pending, or rejected. When it is not-fulfilled, it is either in the "fulfillment is possible", or "fulfillment is impossible" state.

I am still uneasy about the weird behavior of the identity function for a thrown, fulfilled promise, or returning a rejected promise whose reason is a fulfilled promise (which would be used verbatim if reject is not polymorphic). This view of "fulfillment or not" kind of helps, though, since it removes the temptation to think about a rejection as function application on the rejection reason, while it still leaves the opportunity to think of fulfillment as function application on the fulfillment value, which imho, is a good thing.

[domenic]

This is very interesting, and maybe somewhat of a lightbulb moment for me.

Hah, fair's fair :)

It seems like what MarkM is saying is that we can look at a promise as being either fulfilled or not-fulfilled, which is perhaps subtly different from fulfilled, pending, or rejected. When it is not-fulfilled, it is either in the "fulfillment is possible", or "fulfillment is impossible" state.

That's a really nice way of putting it, and I agree.

I am still uneasy about the weird behavior of the identity function for a thrown, fulfilled promise, or returning a rejected promise whose reason is a fulfilled promise (which would be used verbatim if reject is not polymorphic).

To be fair, so I am I. I take consolation in the fact that it's a crazy-weird edge case, as @kriskowal keeps emphasizing.