tc39 / proposal-cancelable-promises

Former home of the now-withdrawn cancelable promises proposal for JavaScript
Other
375 stars 29 forks source link

A new cancellation proposal without a third state #19

Closed bergus closed 8 years ago

bergus commented 8 years ago

Hi, I've followed the discussions with interest but have been too quiet for too long. I'm glad cancellation is on the scope of TC39 again :-)

I've already sketched out my thoughts in this gist that concerned how then should handle cancellation (under the assumption that cancellability should be the default). In the light of generators-as-coroutines this integrates quite nicely with .return() semantics imo. But now I'm reading about third state and am quite disappointed. I have to say I disagree with the whole document :-( But not because it's wrong or just bad, but I believe it's based on a misleading premise:

The question is more about once a promise gets canceled, how that state is represented and propagates down to any derived promises.

Why discuss this question? Why do you believe that cancellation should propagate to derived promises?

I've read the discussion in #10 about immutability. I understand the arguments of both sides, and see the advantages of cancellation tokens being the only way to signal an upstream request for cancellation - tokens that can be passed around separately from the promises, so that only the issuer of a token can trigger the cancellation. It's similar to the distinction between promises and deferreds.

However I believe that there is a simpler solution - and a more intuitive one. Most implementations of cancellability implemented a .cancel() method on cancellable promises, because whoever holds the promise, who requested the result, is usually the same who will want to dispose it, disregarding the result. This signal of disinterest will propagate upwards. Yes, there are exceptions, usually cases where a promise has multiple subscribers (e.g. when caching), but those promises that should not be cancellable are the special case. I believe that "ignore the result" is the most reasonable default for asynchronous actions that cannot be stopped - and it does work out of the box for chains of these, without interspersing token.throwIfRequested() (or if (token.requested) throw.cancel token.reason or whatever it is) between every two awaits - see #18. @domenic arguments there that you'd usually be working with already-cancellable async operations, and you just had to pass them the token. But that would mean passing the token to every single call, which hardly is what we want. It's too easily forgotten (and omitting it on purpose is non-obvious). Why wouldn't we just make it an implicit parameter, and weave it into await/then? Ignoring the result means not having then callbacks being called at all any more. When you cancel a promise, you clean up after you and then expect not to get any (fulfillment) result or (rejection) reason in the future. And in fact that's exactly what .return() does on generators - it does skip everything but finally clauses. This is a very natural equivalence imo - and of course you're discussing this as well.

A return abrupt completion is transformed into a normal completion on the function call boundary, it does not propagate in the way we want cancelation to propagate.

But what if we didn't want cancellation to propagate downwards? If we treat cancellation as a signal of disinterest, it follows that the executor of a promise is not notified until all subscribers have signalled their disinterest (and cancelled the scheduling of their respective callbacks). So when the execution of an async function is interrupted with a return abrupt completion, there is nobody who would be listening to the return value anyway. The problem what to do with unexpected completions from the finally clauses (returning other values, thrown exceptions, further await statements) remains of course, but that's a different discussion. So reconsidering the example

async function inner() {
  try {
    await somethingThatTakesItsTime;
    console.log("(A) this will not run");
  } finally {
    console.log("(B) this will run");
  }
}
async function outer() {
  const p = inner();
  const v = await p;
  console.log("(C) this won't run");
}
const p2 = outer();
p2.cancel();

Here both p2 is cancelled, so it will never fulfill with anything. The C log won't run, as the await resumes with a return abrupt completion. And p will be implicitly cancelled as well, so A won't run either, but B will. The problem with "unwinding the stack so that subsequent work is skipped by not only the caller, but the caller's caller" is solved by propagating the signal of disinterest upwards. And we really don't need a new type of completion value - one that is very much an oddball and has semantics that I've never seen in any other programming language. We don't need to make JS any weirder :-)

Finally I'm proposing that cancelled promise should enter the rejection state, with the cancellation value as the reason. This doesn't matter much, all subscribers have already unsubscribed. No then callbacks would be invoked, no catch blocks triggered. Alternatively we could just never settle them, but that's more complicated and less manageable. The advantage is that new subscribers who are "late to the party" and await the promise or invoke .then() on it after the work has been aborted will now get an error that the result they're looking for is unavailable.

I think that these semantics are much nicer to work with. Whether they should be the default or an optional choice (Promise subclass, extra syntax like cancellable async function(…) …) is open. I hope I didn't forget any important points, and am hoping for lots of feedback :-)

bergus commented 8 years ago

After checking out the ECMAScript Cancel Tokens proposal, I was curious whether it's possible to implement my draft semantics together with the tokens in a promise subclass (without generators yet). It was simpler than I expected at first, though some minor deviations might be possible. See yourself:

class CancellablePromise extends Promise {
    constructor(executor) {
        let requestCancellation;
        let token = new CancelToken(cancel => {
            requestCancellation = (err) => {
                if (this.registeredTokens.every(rt => rt.requested))
                    cancel();
                return token.requested;
            };
        });
        super((resolve, reject) => {
            function resolveCancellable(val) {
                if (typeof val.then == "function") { // crude thenable detection
                    var p = val.then(resolveCancellable, reject);
                    token.promise.then(() => {
                        if (typeof p.requestCancellation == "function") // crude cancellable detection
                            return p.requestCancellation();
                    });
                } else
                    resolve(val);
            }
            executor(resolveCancellable, reject, ::token.promise.then); // passing onCancel, not throw.cancel
            token.promise.then(reject); // maybe this would need to be synchronous from requestCancellation
        });
        this.requestCancellation = requestCancellation;
        this.registeredTokens = [];
        this.token = token;
    }
    then(ignorableOnFulfill, ignorableOnReject, token) {
        let p = super.then(function onFulfill(res) {
            token.throwIfRequested();
            if (typeof ignorableOnFulfill == "function")
                return ignorableOnFulfill(res);
            return res;
        }, function(err) {
            token.throwIfRequested();
            if (typeof ignorableOnReject == "function")
                return ignorableOnReject(err);
            throw err;
        });
        if (token instanceof CancelToken) {
            token.promise.then(::p.requestCancellation); // actually, p.[[reject]]
        } else {
            token = p.token;
        }
        this.registeredTokens.push(token);
        token.promise.then(this.requestCancellation);
        return p;
    }
    catch(ignorableOnReject, token) {
        return this.then(null, ignorableOnReject, token);
    }
    onCancel(onCancel) {
        this.token.promise.then(onCancel);
        return this;
    }
    finally(fin) {
        return this.onCancel(fin).then(fin, fin);
    }
}

You'll notice that every of these promises has a .token property that keeps its cancellation status, and provides a p.requestCancellation(new CancellationReason(…)) method. There should also be a simple utility method that allows to derive an uncancellable promise from this (I guess much like return super.then(...args) would do) so that the multiple-subscriber-problem is easily handled. Or maybe we'd also get new syntax like a uncancellable { …} block in async functions into which the return abrupt completion does not propagate - and that syntax would be reserved for the few cases where it would really be needed.

inikulin commented 8 years ago

Why discuss this question? Why do you believe that cancellation should propagate to derived promises?

Well, because I don't want some dependency lib to which I passed my promise effectively cancel my whole promise chain.

benjamingr commented 8 years ago

Basically from what I understand Domenic (who is championing the proposal) is not inherently against what you're saying - your proposal is in fact based on third-state except for late subscribers which get a rejected promise (and this is also what bluebird does).

The issue from what I understand is that the TC has discussed it and are not interested in pursuing this route.

bergus commented 8 years ago

@inikulin

I don't want some dependency lib to which I passed my promise effectively cancel my whole promise chain.

Then just pass them a Promise, not a CancellablePromise. But that's the exceptional case I believe, you normally don't pass promises around or hand them out to anyone, you return them to your caller who should be able to cancel everything.

your proposal is in fact based on third-state except for late subscribers which get a rejected promise

Yes indeed, though I think I should emphasise that the third state is for the callbacks (onFulfilled called, onRejected called, and neither called) not for the promise. That way I am avoiding a new kind of promise state, and show a way to do it without introducing a new kind of abrupt completion to the language - which the TC didn't really like either it seems ("ARB: The synchronous third state freaks me out. This needs more discussion. We are suddenly adding a totally new continuation across the whole language.") The semantics I'm proposing also allow the transpilation of cancellable async functions to ES6 generators, which imo is a huge pro.

From what I understand is that the TC has discussed it and are not interested in pursuing this route.

Aww. Is that so, is there any official statement on this? I have to admit I skipped over some of the meeting notes and only yesterday read the ones about @domenic's talk on May 25th.

We should pursue both routes, they complement one another well. Cancellation tokens are relatively easy to implement (and polyfill) and provide granular control over cancellation capabilities. They are however awkward to work with, see zenparsing/es-cancel-token#2 and #3. For async/await we want (and afaik the community expects) ignore semantics - e.g. like above, passing a cancel token as the third argument to then that prevents the callbacks from being invoked when it is cancelled, and implicit (upward) propagation of cancellation when chaining - iirc this is better known as Tasks. Switching back and forth between those two sides of the coin should be effortless. I would envision both Promises and Tasks to have utility methods for conversion between each other, e.g.

domenic commented 8 years ago

Hi @bergus, thanks for your interest. As briefly alluded to above, the TC is not interested in pursuing a route with a promise.cancel or task.cancel method in the standard. One contingent is opposed to making the base promise type modifiable by its consumers, and another contingent is opposed to splitting the universe into two separate types of objects (promises and tasks). Between these two, we are forced away from such a design, as it will be unable to achieve consensus and advance.

Since this thread seems unlikely to contribute to the proposal being developed here, and is not a concrete issue with its semantics, I'm going to close this thread. Feel free to continue discussing here if you wish, although given how lengthy your posts are, maybe you would be better served by creating your own proposal repo where they could get justice. Perhaps even as a utility library built on top of cancelation tokens, which would be of separate interest regardless of how the standardization process proceeds in this repo.

bergus commented 8 years ago

Thanks for your stance on the situation in the TC. That's quite disappointing :-( We can only hope that the solution we end up with is not worse than those that can't get consensus.

But I'm trying to offer a way out here. I personally would prefer a promise.requestCancel() method that might or might not succeed (and only propagate to a certain level chosen by the producer), but I can understand the worries about that. What if we did not make promises cancellable, but their consumption? Let's give the Promise.prototype.then method a third optional parameter - a CancelToken that when cancelled leads to the callbacks not being ran. This idea imo voids most of your arguments from the Third State document, and invalidates the need for a new abrupt completion type (that I'm very much opposed to, and which seems to give others a headache as well). Rejections would work as well when nobody's listening to them.

So let's ignore my Task-like proposals above where cancellation does propagate implicitly, and consider passing CancelTokens explicitly. This is how it could look like with async/await:

async function load(target, url, token) {
    function.cancelToken = token; // a meta keyword to register the token
    startLoadingSpinner();
    try {
        let res = await fetch(url, function.cancelToken);
        let body = await res.text(function.cancelToken);
        target.innerText = body;
    } catch (e) {
        showUserMessage("The site is down and we won't be displaying the pudding recipes you expected.");
    } finally {
        stopLoadingSpinner();
    }
}
load(…, …, new CancelToken(cancel => cancelButton.onclick = cancel));

As soon as the registered token is cancelled, the current await resolves to a return abrupt completion.

bergus commented 8 years ago

@domenic

given how lengthy your posts are, maybe you would be better served by creating your own proposal repo where they could get justice.

Yes, that might be a good idea. Would you offer to champion such a proposal (once it is mature enough) and present it to the TC as an alternative to the approach detailed in this repo? I'm not sure if the only way to convince the TC of issues with a proposal is to come up with a better one. There doesn't seem to be a documented process to drop a proposal, I've only seen them advance through the stages.

ljharb commented 8 years ago

@bergus https://github.com/tc39/proposals/blob/master/inactive-proposals.md

bergus commented 8 years ago

@ljharb Ah thanks. But it appears those were dropped because of commitee decisions. What kind of community input is expected to let TC39 know of disagreement with some design choices? Is opening Github issues enough, will they be read?

ljharb commented 8 years ago

Certainly opened github issues are read. The best place to open an issue is on the repo for the proposal, however.

domenic commented 8 years ago

@bergus in your latest example it's not clear to me what value function.cancelToken presents. You assign to it (accomplishing what?) but then just pass the value in to everything you call. You should just pass token directly.

It's also unclear what state the promise returned from load, or even from fetch, is in when you click the cancel button. If it is a third "canceled" state, which is not caught by catch but does go through finally, everything works. But you seem to be against the third state for reasons I don't understand, which means I have no idea how the example would work.

bergus commented 8 years ago

it's not clear to me what value function.cancelToken presents

I intended it to be a new meta property (like new.target) that on assignment registers a token with the current async function evaluation so that it knows when it's being cancelled. Functions where there is no such assignment are simply not cancellable, they always run without being stopped in the middle. As a getter, the meta property just returns the value that was previously assigned to it, or if there is none then a token that is never cancelled. This kind of "registration" is necessary when the async function is not allowed to return a Task-like object/promise that can be cancelled from outside (e.g. with a .cancel method) but must receive the token as a parameter. This particular syntax is just bikeshedding, there might be better (more declarative) options.

When a canceltoken is registered with an async function evaluation, every await x will lead to x.then(resume, throw, token) being called, so that resume/throw are not called when the registered token is cancelled. Also a subscription to the token is made that resumes the await evaluation with a return completion (going through finally clauses but not catch ones).

This especially allows to let cancellation break away from await something() even when something does not support taking tokens or returning promises with special states.

It's also unclear what state the promise returned from load, or even from fetch, is in when you click the cancel button.

It doesn't matter any more - the subscription to those promises is cancelled with the registered token.

I intend the promise state to be rejected, so that we don't need to alter semantics of basic promises, and so that any future subscribers (that don't know about the cancellation because they didn't pass tokens to the creation) do get an exception about the unexpected unavailability of the result.

domenic commented 8 years ago

OK. I think the kernel of the idea here is some way of opting in to making await x sugar for await x; cancelToken.cancelIfRequested();. In this way perhaps it is similar to the request in #18. That seems plausible. function.cancelToken = ct or await.cancelToken = ct seems like an OK way to do that.

But the rest of the proposal (adding cancel tokens to .then and moving away from the third state) seems to not solve any new problems, and in fact un-solves existing ones by creating spurious rejections.

bergus commented 8 years ago

opting in to making await x sugar for await x; cancelToken.cancelIfRequested();

More like await Promise.race(x, cancelToken.cancelIfRequested()) I guess, but yes. And I don't want to make it sugar, I want to make it the only way. An alternative syntax might be

await x orGetCancelledBy cancelToken;

adding cancel tokens to .then seems to not solve any new problems

I think it is necessary to solve the problem of a race between resolution and cancellation, see zenparsing/es-cancel-token/issues/7. There might be other solutions, this was just my favourite one. Please join the discussion over there! At least it is a very helpful utility, providing a very simple and powerful notation for writing cancellable chains of then calls. It also provides a useful interface for possible (userland) Task implementations that keep track of their subscriptions implicitly.

moving away from the third state seems to not solve any new problems

I do consider a third state to be a huge problem:

un-solves existing problems by creating spurious rejections

Can you be a bit more specific with that, please? Yes, we can simply keep the promises pending (never settling), and that wouldn't change anything about not needing a third state. We can explore that route if you want.

But I believe putting cancelled promises into a rejected state does solve some problems:

I'll assume you refer to "unhandled cancellations" and that cancellation is not exceptional. You're right, my proposal did not yet include solutions for these (you already said the post was too long :-)). First of all, I don't think that all rejections are exceptional. They're just as often used for control flow in practice. Many rejections are expected, and cancellation will be as well (and be ignored, usually).

Solving the problem of not reporting cancellations to tooling is possible as well. One option is ignoring instances of CancelReason. Another option would be tagging the promises created by x.then(…, …, token) as being intentionally rejected when token is cancelled. We still could pass a third action, the cancel function, to the executor in the Promise constructor for that, it just would lead to rejection instead of a third state. Or we could pass the token as a second argument to the Promise constructor so that it knows when it's being cancelled (and implicitly subscribe the reject action to the token).

bergus commented 8 years ago

I have now fleshed out a new proposal at bergus/promise-cancellation - superseding all the Task-like behviour from the OP here. Please have a look at it!