Skip to content
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

Strawman: Promise Creation and Resolution API #10

Closed
briancavalier opened this issue Jan 16, 2013 · 27 comments
Closed

Strawman: Promise Creation and Resolution API #10

briancavalier opened this issue Jan 16, 2013 · 27 comments

Comments

@briancavalier
Copy link
Member

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:

  • It builds on the new Promise(fn) idea, but simplifies fn's signature to accepting a single resolve function.
  • I did away with fulfill() and reject() in favor of a single resolve(). My current feeling is that fulfill() is not a good thing.
  • It also specifies a method for creating a rejected promise, which is necessary here given how the single resolve() function works.
  • I tried to cover thrown exceptions in the promise creation callback in a way that is similar to what I proposed over in Unspecified behavior when onFulfilled or onRejected throws a promise object promises-spec#65.
  • If we decide to move away from the word "resolve" and its derivatives, I'm ok with that. The word itself is not required to appear in code--e.g. there are no properties named 'resolve'. I'm just using it here because I wanted to get this done, and didn't want to spend time searching for another word.

Let's discuss :)

Promise Creation and Resolution API

new Promise(resolutionFunction)

var p = new Promise(resolutionFunction);

// where
function resolutionFunction(resolve) {
    // Do work, and then
    resolve(valueOrPromise);
    // or
    // throw exception;
}

Creates a new, pending promise whose fate can be sealed by calling the resolve function with a value or another promise. After resolve(valueOrPromise) is called,

  1. if valueOrPromise is not a promise, p will be transitioned to the fulfilled state, and its fulfillment value will be valueOrPromise
  2. if valueOrPromise is a fulfilled promise, p will be transitioned to the fulfilled state, and its fulfillment value will be valueOrPromise's fulfillment value
  3. if valueOrPromise is a rejected promise, p will be transitioned to the rejected state, and its rejection reason will be valueOrPromise's rejection reason
  4. if valueOrPromise is a pending promise,
    1. after valueOrPromise is fulfilled, p will be transitioned to the fulfilled state, and its fulfillment value will be valueOrPromise's fulfillment value
    2. after valueOrPromise is rejected, p will be transitioned to the rejected state, and its rejection reason will be valueOrPromise's rejection reason
  5. if resolutionFunction throws an exception (let e be the thrown exception),
    1. if e is not a promise, p will be transitioned to the rejected state, and its rejection reason will be e.
    2. if e is a fulfilled promise, p will be transitioned to the rejected state, and its rejection reason will be e's fulfillment value
    3. if e is a rejected promise, p will be transitioned to the rejected state, and its rejection reason will be e's rejection reason
    4. if e is a pending promise,
      1. after e is fulfilled, p will be transitioned to the rejected state, and its rejection reason will be e's fulfillment value
      2. after e is rejected, p will be transitioned to the rejected state, and its rejection reason will be e's rejection reason

Promise.reject(valueOrPromise)

var p = Promise.reject(valueOrPromise);

Creates a new, eventually rejected promise.

  1. if valueOrPromise is not a promise, p's rejection reason will be valueOrPromise
  2. if valueOrPromise is a fulfilled promise, p's rejection reason will be the valueOrPromise's fulfillment value.
  3. if valueOrPromise is a rejected promise, p's rejection reason will be the valueOrPromise's rejection reason.
  4. if valueOrPromise is a pending promise,
    1. after valueOrPromise is fulfilled, p will be transitioned to the rejected state, and its rejection reason will be valueOrPromise's fulfillment value
    2. after valueOrPromise is rejected, p will be transitioned to the rejected state, and its rejection reason will be valueOrPromise's rejection reason
@briancavalier
Copy link
Member Author

Made a couple very minor edits, mainly reducing bullet nesting.

@briancavalier
Copy link
Member Author

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.

@juandopazo
Copy link

Do we want to trap errors in the init function? It looks like we should be letting it break early.

@ForbesLindesay
Copy link
Member

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.

@briancavalier
Copy link
Member Author

@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()

@juandopazo
Copy link

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.

@briancavalier
Copy link
Member Author

Perhaps "new Promise(resolutionFunction)" bullet 5 can be rewritten to something like:

  1. 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.

@ForbesLindesay
Copy link
Member

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.

@domenic
Copy link
Member

domenic commented Jan 18, 2013

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.

@domenic
Copy link
Member

domenic commented Jan 18, 2013

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.

@briancavalier
Copy link
Member Author

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.

@ForbesLindesay
Copy link
Member

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.

@domenic
Copy link
Member

domenic commented Jan 21, 2013

@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.

@kriskowal
Copy link
Member

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.

@ForbesLindesay
Copy link
Member

@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.

@juandopazo
Copy link

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.

@briancavalier
Copy link
Member Author

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.

@briancavalier
Copy link
Member Author

@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.

@domenic
Copy link
Member

domenic commented Jan 23, 2013

I thought I'd been clear, but let me try again:

  • Fulfilled and rejected are the two "settled" states of a promise. A promise is either fulfilled, rejected, or pending.
  • A call fulfill(x) should fulfill the promise with the fulfillment value x. A call to reject(x) should reject the promise with rejection reason x.
  • Relatedly, 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.
  • This means 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 promises-aplus/promises-spec#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!

@domenic
Copy link
Member

domenic commented Jan 23, 2013

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 :).

@kriskowal
Copy link
Member

The more we talk about this, the better I like resolve and reject the way they are presently implemented, despite the pedagogical hiccups.

@ForbesLindesay
Copy link
Member

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.

@ForbesLindesay
Copy link
Member

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.

@briancavalier
Copy link
Member Author

@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.

@domenic
Copy link
Member

domenic commented Jan 27, 2013

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.

@ForbesLindesay
Copy link
Member

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.

@briancavalier
Copy link
Member Author

Since this has a polymorphic reject, it really has no chance of being accepted, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants