Closed briancavalier closed 11 years ago
Made a couple very minor edits, mainly reducing bullet nesting.
Open issue: It seems clear how to handle the case of resolutionFunction
throwing an exception before resolve()
is called, but not so clear how to handle the case of it throwing after resolve()
has been called.
Do we want to trap errors in the init function? It looks like we should be letting it break early.
I think we want to trap the errors either way. If resolve has already been called, we should silence the error. If resolve has not been called we should transform the error into a rejection.
@ForbesLindesay Interesting. My knee jerk was the opposite: if resolutionFunction
throws after resolve()
has already been called, we should silently swallow whatever effect resolve()
would have had, and transform the exception into a rejection (using the algorithm in "new Promise(resolutionFunction)" bullet 5).
Hmmm, hard to know which is better: favoring the resolve()
or favoring an exception thrown after resolve()
Errors are not something to ignore. I'd say we treat it the same way we do in "then". If the promise was already completed, then throwing does nothing. If not, it rejects it.
Perhaps "new Promise(resolutionFunction)" bullet 5 can be rewritten to something like:
5) if resolutionFunction
throws an exception (let e
be the thrown exception), p
will become the equivalent of Promise.reject(e)
(see Promise.reject(valueOrPromise) below).
Hmmm, or maybe it's better to leave it spelled out fully.
Ah, I hadn't considered the fact that the entire function is synchronous, I like @briancavalier's initial instinct to ignore the resolve call and just turn it into a rejection if an exception was thrown. The only downside is that we can't do that when the resolution/rejection is a asynchronous.
I am glad we are moving forward with this. I am generally happy with the idea here, and am even coming around to keeping the "resolve" verb, in favor of renaming the adjective. As long as we can get Mark Miller on board with a new adjective too, we're in good shape. But let's put aside the naming, as you said.
I don't like Promise.reject
. I think using constructor functions as namespaces is a bit annoying, and e.g. Q would then have to do something like Q.reject = Promise.reject
, resulting in confusion. I like the {resolve.reject
, resolve.fulfill
} idea best, as it clearly shows how resolve is the kind of meta-operation that encompasses the two basic operations from the original Promises/A+ spec.
Furthermore, I think the algorithm for Promise.reject
is bad. It should just reject with the reason given, or we should find some other name for the verb. "Reject" is in the same basic-operation category as "fulfill", and as such, you can reject or fulfill with any reason: promises are not prohibited. "Resolve" is the thing that has special behavior with promises.
As for the thrown-exception vs. resolve
-call thing, I think throwing should essentially be equivalent to calling reject. That is:
// should be fulfilled with 5
var p = new Promise(function (resolve) {
resolve(5);
throw new Error();
});
// should be rejected with the error
var p = new Promise(function (resolve) {
setTimeout(resolve.bind(null, 5), 100);
throw new Error();
});
This isn't quite the same as @briancavalier's last comment, though. In particular "p
will become..." seems like it will always happen, i.e. doesn't match the first case.
It's a bit annoying that we can't allow returning from the function to resolve with that value, since it implicitly returns undefined
. That breaks the symmetry with how then
's arguments are handled.
Ah, it just occurred to me why exactly I find Promise.reject
annoying. It's actually not necessary to have a distinguished way of creating rejected promises, any more than it is to have a way of creating fulfilled ones. That is:
var fulfilledPromise = new Promise(function (resolve) { resolve(5); });
var rejectedPromise = new Promise(function () { throw 5; });
or even more bluntly, Promise.reject
is redundant because of
Promise.reject = function (reason) {
return new Promise(function() { throw reason; });
};
Thus the truly minimal proposal doesn't include it at all, leaving me in a good position to be in agreement with the minimal and strongly in favor of resolve.fulfill
,resolve.reject
.
Cool, we seem to be converging on at least new Promise(function(resolve) { ... })
where resolve
acts like 1-4 in the strawman.
Promise.reject
@domenic You're right, Promise.reject
isn't necessary. However, one problem with only relying on throw
is that it means the only truly safe, cross-library way to reject asynchronously from within resolutionFunction
is, IMHO, non-obvious and quite hard on the eyes:
new Promise(function(resolve) {
setTimeout(function() {
resolve(new Promise(function() {
throw new Error();
}));
});
});
Nobody will want to type that. I def don't want to type that :) Implementations will inevitably provide sugar for it, so why not just include some now so that at least the implementations will all have the same sugar. To quote @unscriptable: "It's like ice cream, just put the sugar in there cuz nobody wants it without it"
So, even though it's not absolutely minimal, I feel like we should specify either: 1) a terse way of creating an eventually rejected promise, which can be passed to resolve
, or 2) a resolve.reject
function, provided neither of these can leak promises to handlers.
fulfill(verbatim) and reject(verbatim)
I've documented my reasons here and here for why I feel leaking promises to handlers is dangerous. Mark Miller and @kriskowal have expressed similar concerns, at least about fulfill
.
resolutionFunction return values
This would be a very nice parallel with then
, but I agree with @domenic that it seems impossible to allow, since functions without a return
will implicitly return undefined
, and undefined
is a legal fulfillment value. We can't distinguish between a function that intended to return undefined as the fulfillment value and one that intended not to return something but to call resolve
asynchronously.
Promise.reject
I think your code can be safely simplified to:
new Promise(function() {
throw new Error();
});
since then
is always async. In addition, say I want to create a promise using library Foo
but I don't know if Foo
has any reject
helper, but I know Q
does:
new FooPromise(function (resolve) {
realAsyncOp(function (err) {
resolve(Q.reject(err));
});
});
If anything I think it would also be worth being able to do:
new FooPromise(function (resolve) {
realAsyncOp(function (err) {
resolve.reject(err);
});
});
as that will save the creation of an un-necessary promise (and therefore may perform significantly better).
resolutionFunction return values
We could handle resolutionFunction return values that are promises (but not that are values). This would be helpful for the fairly common scenario:
function doSomething() {
return new Promise(function () {
var foo = someSyncOperationThatMightThrow();
return asyncOperation(foo);
});
}
You need to do the sync stuff inside the promise wrapper so you can catch thrown exceptions, but after that you can just return a promise.
@briancavalier
I feel strongly that we need fulfill
and reject
verbatim, even if they are recommended against and problematic. They are the fundamental terms of the Promises/A+ spec, and saying people should abandon them in favor of resolve
is not going to fly.
In essence, we have painted ourselves into a corner, and need to start living with it. That corner being the usage of polymorphic return
, which leads to the identity-function-breakage you eloquently explain.
We can encourage the use of resolve
over fulfill
, and indeed, by making fulfill
a property of resolve
, we do just that. But we cannot fix reject
: the boat has already sailed. Reject is already defined as the counterpart to fulfill, and both take a value, with no special semantics for promises.
You could try to introduce a fourth verb, call it polymorphicReject
, and attempt to evangelize the use of it. But that seems unlikely to work at this stage. Besides, it has all the same confusion of resolve
: if you polymorphicReject(pendingPromise)
, the promise in question will not be rejected until pendingPromise
is settled. And it's even less useful, since rarely do you want to reject no matter what---you usually want to fulfill if pendingPromise
fulfills, i.e. you want resolve
behavior.
I think the best we can get at this stage is a polymorphic resolve
that people mostly use by default, alongside non-polymorphic resolve.fulfill
and resolve.reject
that don't have as nice properties in regard to identity function breakage, but are actually intuitively graspable and predictable, unlike resolve
or polymorphicReject
. We can then assume that people will take the path of least resistance and use resolve
instead of fulfill
, giving us in most cases what we want.
@ForbesLindesay
I'm on the fence about allowing return values that are promises to have an effect. It is nice, but it is also misleading, and doesn't fit well with one's expected intuition, IMO.
We must ignore the return value of the promise constructor. Allowing a promise return value would imply using resolve(returned), which would imply that omitting a return value would resolve the promise to undefined, which would clearly not fly. To make it work, we would have to special case either undefined
or ignore non-promises. Either way, it confuses the contract of the promise constructor: call resolve.
@kriskowal I agree, I'm not convinced by return values either, I was just making sure we considered something that could work (and would have some value).
@domenic
I'm not convinced we need a verbatim fulfill
method. I think it might be better not to spec it as part of resolvers-spec and leave it as an extension that's provided by some of the libraries.
As for reject, that boat sadly has sailed. Unless we can find another nice word that means polymorphicReject
so we can do what JavaScript did with var
=> let
.
A passing comment to give this discussion a little context. I recently had an interesting talk with a friend who writes Haskell in which he told me that monads have two distinct operations: map
and bind
.
promise.then(aFunctionThatReturnsAValue)
would be map
.
promise.then(aFunctionThatReturnsAPromise)
would be bind
.
I think they got it right that way. Having different operations (functions, methods) for returning values or promises makes all this discussion go away. Of course we're a bit late to go in that direction, but it's interesting to keep it in mind.
I agree with @ForbesLindesay's suggestion that we simply don't specify fulfill
. If a library opts to provide it and can still meet all the requirements of our specs, then that seems perfectly fine to me, as long as the library implementor understands the tradeoffs, and is aware that it may cause some confusion for users.
@domenic and @ForbesLindesay I'm not clear on why the boat has sailed for reject(verbatim)
. Is it common for people to use it to thread promises through to a rejection handler? I've not seen it, but if people are doing it, I agree we need to consider that as a data point.
One data point is that when.reject(x)
, as it exists today, behaves like polymorphicReject
, and we've not had any complaints.
Another data point, although not directly related to reject(verbatim)
is that many promise implementations have a verb named "resolve" that behaves like "fulfill". So, they'll need to change or be made non-compliant if we specify a polymorphic resolve. It seems no worse to specify that reject be polymorphic also.
The primary problem with resolve
being "special" was that it may ultimately lead to a fulfilled or rejected promise based on the eventual outcome of its input. That is not the case for a polymorphic reject()
: A polymorphic reject would always lead to an eventual rejection.
I am definitely sensitive to the fact that we have to be careful not to break the world. However, if there's little or no evidence that we would, then it feels like we should look at "reject" in a more forward-looking light. Breaking the identity function and creating hazards where simply returning an input swallows a rejection are both pretty serious downsides in my mind, and I don't have a clear picture of what the upsides of reject(verbatim)
are.
@juandopazo Yeah, that's a good point. Unfortunately, I think you're right that we can't pursue it without breaking the world, since then
's behavior is so established.
I thought I'd been clear, but let me try again:
fulfill(x)
should fulfill the promise with the fulfillment value x
. A call to reject(x)
should reject the promise with rejection reason x
.fulfill(x)
should put the promise in a fulfilled state, no matter the value of x
. Similarly, reject(x)
should put the promise in a rejected state, no matter the value of x
.fulfill(x)
and reject(x)
cannot be polymorphic, because if x
is a forever-pending promise, the above will be violated.Anything that breaks the direct connection between the states fulfilled and rejected, and the verbs fulfill and reject, is not acceptable.
Note that the above arguments intentionally tie the existence and survival of fulfill
and reject
together. What goes for one, goes for the other. This is the path we have laid down, and now we must walk it!
Thus, the only solution I see if we truly want your semantics, @briancavalier, is a new verb (polymorphicReject
). Given the new prominence of this polymorphic reject algorithm over in https://github.com/promises-aplus/promises-spec/issues/65, this even might be worthwhile.
One idea I hesitate to even pose: rename "rejected" to "broken" in the base Promises/A+ spec, and use "reject" as the name for polymorphicReject
. Pro: seems to solve the issue. Con: now we have twice as many confusing concept-pairs! Resolve/fulfill isn't confusing enough, so let's add reject/break!
Just realized something:
@briancavalier, in essence your point seems to be that promises-for-promises are hazardous, which I sort-of agree with. My question: does having only resolve
and polymorphicReject
, with no simple fulfill
/reject
, prevent promises-for-promises from ever being created? If so, that would be a pretty valuable invariant, probably worth the pedagogical cost.
I feel like this was a lightbulb moment, guys :).
The more we talk about this, the better I like resolve
and reject
the way they are presently implemented, despite the pedagogical hiccups.
Yes, as far as I'm aware you'd never be able to create a promise for a promise if we did that. You could of course crete a promise for an array containing a promise (Q.resolve([myPromise])
) but that seems reasonable.
P.S. I'd be inclined to pay the cost of making reject
have the semantics proposed for throw
in the promises-spec and using broken
to be the parallel of fulfill
. I'd then suggest not specifying fulfill
or broken
in the resolvers spec.
This is under the assumption that almost nobody is currently attempting to reject with a promise.
@domenic and @ForbesLindesay Yeah, I think along with polymorphic throw
behavior (and the existing polymorphic return
), it's sufficient. Captain obvious here, but: it follows that a promise will never be passed verbatim as an onFulfilled
or onRejected
arg.
Wait a minute. If they throw in the factory function, we can just let that happen. Since the factory function is called synchronously, it'll crash the program. We don't have to transform it into reject at all.
no, but it's a real pain if we don't. As you mentioned with not wanting .then
to ever throw synchronously. The exact same argument applies here.
Consider this function:
function doAsyncWork() {
return new Promise(function (resolver) {
var resA = doSynchronousPreparationWhichMightThrowAnException();
resolver.resolve(doAsyncWork(resA));
});
}
If you want to call it and be sure of handling all errors you need code to handle both synchronous and asynchronous errors, this is really bad.
Since this has a polymorphic reject, it really has no chance of being accepted, so closing.
After thinking about all of this a bit more, I wrote down this strawman for what I feel is a minimum API for promise creation and resolution. First, a few notes:
new Promise(fn)
idea, but simplifiesfn
's signature to accepting a singleresolve
function.fulfill()
andreject()
in favor of a singleresolve()
. My current feeling is thatfulfill()
is not a good thing.Let's discuss :)
Promise Creation and Resolution API
new Promise(resolutionFunction)
Creates a new, pending promise whose fate can be sealed by calling the
resolve
function with a value or another promise. Afterresolve(valueOrPromise)
is called,valueOrPromise
is not a promise,p
will be transitioned to the fulfilled state, and its fulfillment value will bevalueOrPromise
valueOrPromise
is a fulfilled promise,p
will be transitioned to the fulfilled state, and its fulfillment value will bevalueOrPromise
's fulfillment valuevalueOrPromise
is a rejected promise,p
will be transitioned to the rejected state, and its rejection reason will bevalueOrPromise
's rejection reasonvalueOrPromise
is a pending promise,valueOrPromise
is fulfilled,p
will be transitioned to the fulfilled state, and its fulfillment value will bevalueOrPromise
's fulfillment valuevalueOrPromise
is rejected,p
will be transitioned to the rejected state, and its rejection reason will bevalueOrPromise
's rejection reasonresolutionFunction
throws an exception (lete
be the thrown exception),e
is not a promise,p
will be transitioned to the rejected state, and its rejection reason will bee
.e
is a fulfilled promise,p
will be transitioned to the rejected state, and its rejection reason will bee
's fulfillment valuee
is a rejected promise,p
will be transitioned to the rejected state, and its rejection reason will bee
's rejection reasone
is a pending promise,e
is fulfilled,p
will be transitioned to the rejected state, and its rejection reason will bee
's fulfillment valuee
is rejected,p
will be transitioned to the rejected state, and its rejection reason will bee
's rejection reasonPromise.reject(valueOrPromise)
Creates a new, eventually rejected promise.
valueOrPromise
is not a promise,p
's rejection reason will bevalueOrPromise
valueOrPromise
is a fulfilled promise,p
's rejection reason will be thevalueOrPromise
's fulfillment value.valueOrPromise
is a rejected promise,p
's rejection reason will be thevalueOrPromise
's rejection reason.valueOrPromise
is a pending promise,valueOrPromise
is fulfilled,p
will be transitioned to the rejected state, and its rejection reason will bevalueOrPromise
's fulfillment valuevalueOrPromise
is rejected,p
will be transitioned to the rejected state, and its rejection reason will bevalueOrPromise
's rejection reason