tc39 / proposal-async-await

Async/await for ECMAScript
https://tc39.es/proposal-async-await/
Apache License 2.0
1.58k stars 106 forks source link

How to cancel a running async function #27

Closed mnieper closed 9 years ago

mnieper commented 9 years ago

Is it possible to short-circuit a running async function like one can do with a generator by calling its return method?

A use case would be to have a finally block invoked in order to clean up resources, etc.

bmeck commented 9 years ago

in a similar vein we have a lot of code that relies on errors coming from outside sources, but I can't find a way to abort a running async function:

async function () {
  // ... get a child process ...

  // caused by some things that do not actually kill the process
  // e.g. stdio causing SIGPIPE
  child_process.on('error', abort);

  // ... do things until .on('exit') ...
}

for a lot of things like streams, where errors are disjoint (not in the same stack as a pending callback) it would be nice to have a clear way to abort.

steelbrain commented 9 years ago

Throw an error maybe? Instead of aborting.

getify commented 9 years ago

I'd consider this to be a blocking issue in the current design. If the OP case wasn't addressed (aborting from outside), I would never use async functions and would just stick with the slightly uglier generator+promise option with a runner util.

A possible idea to address:

async function foo() { .. }

var ret = foo();
ret.value;       // promise
ret.return(..);  // analogous to generator `return(..)`
                 // returns either its own separate promise
                 // or the original `ret.value` promise

so...

foo().value.then(..) 

// instead of

foo().then(..)

Not wonderful, but certainly workable compared to no solution.

petkaantonov commented 9 years ago

@getify How does .return() as specified not lead to resource leaks and inconsistent state?

function* () {
    // Set up state and resources
    try {
        yield thing1();
        yield thing2(); // Say you cancel while waiting for this...
        yield thing3();
    } finally {
        // Cleanup
    }
}

If you just cancel that generator with .return() it will leave behind resources and inconsistent state?

mnieper commented 9 years ago

The finally block will be executed by .return().

2015-02-19 23:48 GMT+01:00 Petka Antonov notifications@github.com:

@getify https://github.com/getify How does .return() as specified not lead to resource leaks and inconsistent state?

function* () { // Set up state and resources try { yield thing1(); yield thing2(); // Say you cancel while waiting for this... yield thing3(); } finally { // Cleanup } }

If you just cancel that generator with .return() it will leave behind resources and inconsistent state?

— Reply to this email directly or view it on GitHub https://github.com/lukehoban/ecmascript-asyncawait/issues/27#issuecomment-75155651 .

petkaantonov commented 9 years ago

@mnieper nothing about that in the es6 spec, and v8 doesn't implement it so cannot test

mnieper commented 9 years ago

See here

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-generator.prototype.return

which relies on

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-generatorresumeabrupt

which in turn resumes a suspended generator in step 11.

Now look at

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-generatorresumeabrupt

where the semantics of yield are given.

-- Marc

(Traceur implements all this.)

2015-02-20 8:32 GMT+01:00 Petka Antonov notifications@github.com:

@mnieper https://github.com/mnieper nothing about that in the es6 spec, and v8 doesn't implement it so cannot test

— Reply to this email directly or view it on GitHub https://github.com/lukehoban/ecmascript-asyncawait/issues/27#issuecomment-75200062 .

getify commented 9 years ago

As a side note, TBH, I've read those several times before, and I'm still not clear exactly where in there:

  1. the try..finally resumes, and then the finally is given a chance to complete
  2. what happens when a yield is encountered inside the finally.

I don't know if BabelJS is fully up to spec on this return(..) stuff or not, but: bablejs generator yield return finally yield

sebmck commented 9 years ago

@getify Babel uses regenerator for generators and async functions. Also important to note that this is also the behaviour of Traceur.

mnieper commented 9 years ago

If a yield is encountered inside the finally block, the generator resumes its normal state.

The return value is saved and returned later (as long as no other return happens inside the finally block).

See also here: https://bugzilla.mozilla.org/show_bug.cgi?id=1115868

2015-02-20 16:48 GMT+01:00 Sebastian McKenzie notifications@github.com:

@getify https://github.com/getify Babel uses regenerator https://github.com/facebook/regenerator for generators and async functions. Also important to note that this is also the behaviour of Traceur https://google.github.io/traceur-compiler/demo/repl.html#function%20*foo()%20%7B%0A%20%20try%20%7B%0A%20%20%20%20yield%201%3B%0A%20%20%7D%0A%20%20finally%20%7B%0A%20%20%20%20yield%202%3B%0A%20%20%7D%0A%20%20yield%203%3B%0A%7D%0A%0Avar%20it%20%3D%20foo()%3B%0A%0Aconsole.log(%20it.next()%20)%3B%0Aconsole.log(%20it.return(4)%20)%3B%0Aconsole.log(%20it.next()%20)%3B%0Aconsole.log(%20it.next()%20)%3B%0A .

— Reply to this email directly or view it on GitHub https://github.com/lukehoban/ecmascript-asyncawait/issues/27#issuecomment-75259707 .

bmeck commented 9 years ago

It is the correct behavior, even if it is strange. Just like how finally may never be invoked if the generator yields during a try block but never gets resumed. Strange things did happen here. Lots of guarantees have been thrown out w/ yield.

On Fri, Feb 20, 2015 at 9:48 AM, Sebastian McKenzie < notifications@github.com> wrote:

@getify https://github.com/getify Babel uses regenerator https://github.com/facebook/regenerator for generators and async functions. Also important to note that this is also the behaviour of Traceur https://google.github.io/traceur-compiler/demo/repl.html#function%20*foo()%20%7B%0A%20%20try%20%7B%0A%20%20%20%20yield%201%3B%0A%20%20%7D%0A%20%20finally%20%7B%0A%20%20%20%20yield%202%3B%0A%20%20%7D%0A%20%20yield%203%3B%0A%7D%0A%0Avar%20it%20%3D%20foo()%3B%0A%0Aconsole.log(%20it.next()%20)%3B%0Aconsole.log(%20it.return(4)%20)%3B%0Aconsole.log(%20it.next()%20)%3B%0Aconsole.log(%20it.next()%20)%3B%0A .

— Reply to this email directly or view it on GitHub https://github.com/lukehoban/ecmascript-asyncawait/issues/27#issuecomment-75259707 .

getify commented 9 years ago

@mnieper i know that's what it does, I was saying I haven't yet been able to fully explain that by the wording I see in the spec. I've read through it carefully several times, and I still keep getting lost.

getify commented 9 years ago

btw, I just threw together this quick ugly diagram illustrating my point about how a cancelable async function would fit:

cancelable async

The main takeaway is that this doesn't rely on artificially injecting cancelation through shared scope (i.e., using a side-effect to tell step 2 to abort), nor does it rely on promise cancelation (action at a distance).

It instead treats observation and cancelation as separate capabilities (much like early promise/deferreds did, btw!), which only the initiating code (main() in the diagram) receives, leaving the promise to be shared with others in a purely read-only observation role.

bmeck commented 9 years ago

other use case that is fairly important that we are using from generator-runner code:

async function () {
  try {
     // init cleanup directives
     let lock = await lockfile(file);
     lock.oncompromised = cancel;
     // stuff that should stop if lock becomes compromised
  }
  finally {
    // cleanup
  }
}
lukehoban commented 9 years ago

I think this question is really just about what the de facto or de sure standard will be for Promise cancellation.

I personally think a token-based cancellation model that is passed in separately from the Promise instances is a good option for this, that also works great with async/await (see C# precedent for what this looks like at scale).

But I'm not sure there is anything that can/should be changed in the async/await proposal separate from what needs to be done in furthering the models for cancellation around Promise-based APIs.

bmeck commented 9 years ago

@lukehoban my concern is when you need to pass the ability to cancel to APIs in use by the async function like the lockfile example I put up.

getify commented 9 years ago

Having to pass around a cancelation token definitely harms the composability of many async functions, unless some de facto standard emerges where the first (or last) argument is always this token, kinda like the ugly "error-first" style of callbacks in node.

Cancelation of promises is way, way far away from a given. A lot of high-visibility folks are already assuming it'll happen, but there are others (like me) who are ready to die on a sword to try to stop that from happening.

This is not as simple as just "wait for promises to figure it out". Promises shouldn't be cancelable. If that means that async functions shouldn't / won't be cancelable, fine. Just say that. It means some of us won't use them, at least not in all cases, but that's a better outcome than just following the cancelation crowd.


Consider this for a moment: let's say that promises do become cancelable. What does that mean for a running (paused) async function instance if such a signal were somehow received? Is it going to end up doing something almost like what generators do, where it aborts the running context, but then still lets any waiting finally's run? If that's true, then why not actually follow generator's lead design wise and go the direction of having an async function return a controller (as I suggested earlier in the thread) of some sort, which includes both the promise and an abort/cancel/whatever trigger?

This spec could take the lead and come up with something that makes more sense than cancelable promises, and then maybe the fetch(..) crowd could follow that. I tried providing feedback into the fetch(..) thread, but my concerns were mostly disregarded. I have a naive hope that maybe this spec/process will have a more sensible outcome.

bmeck commented 9 years ago

I would just like to reinforce, it is not always the outside (where a controller would be) that wants to be able to stop execution, sometimes conditions that compromise a task need to be hooked up inside of the function itself. For this use case I am not sure promise cancellation is even a _viable_ solution, though things like generator's .return are how we are doing this right now.

getify commented 9 years ago

@bmeck perhaps I missed it, but if the cancelation signal needs to come from inside the async function, why can't throw (from inside a try..finally) work?

bmeck commented 9 years ago

@getify timing is the issue, right now you can throw which will work if you do not have out of band cancellation.

In my example, we need to allow the lock to fire a callback to cancel our function. This is because the lockfile library is the authority on _when_ the lock is compromised. If we want to emulate this with try/catch/finally we would need any await to be followed with a check for any cancellations. This would be more akin to return than throw.

getify commented 9 years ago

out of band cancellation.

Nods. I guess I was assuming all such out-of-band-cancelation could be given access to the controller. which, as you said, already maps to essentially what you're doing now. I can't envision another way of that working.

getify commented 9 years ago

Just wanted to add another piece of information in support of my suggested "controller" object pattern: Revocable Proxy.

Proxy.revocable(..) returns an object with two properties, the proxy itself and the revoke() function. This is exactly analogous to what I'm suggesting should be the return value from an async function, except instead of revoke() we should call it return() or cancel(), and promise instead of proxy.

Object destructuring makes this still super easy to deal with:

var {promise:p1} = asyncFn();

var {promise:p2,cancel} = asyncFn();
// later... `cancel()`
bterlson commented 9 years ago

I'm closing this for now. Once the library space figures out how cancellation should work we can start talking about syntax.

felixfbecker commented 9 years ago

Imo promises and async/await is an abstraction to make async code look sync. 99% of times it will be enough - like replacing node errbacks. If the abstraction doesn't fit the use case, than something else than promises (and therefor async/await) should be used, like observables. Ben Lesh gave a great talk about observables at ng-europe last month and why promises just don't work for ajax requests, for example because they can't be canceled. https://www.youtube.com/watch?v=KOOT7BArVHQ

RangerMauve commented 9 years ago

I'm totally in agreement that if you want it to be cancellable, that promises aren't the correct primitive to use, and therefore async functions aren't the correct primitive to use.

getify commented 9 years ago

async functions don't have to return promises. that's a choice (and a bad one IMO). this is tail-wagging-the-dog behavior to work backwards from "i want to cancel" to "promises shouldn't be cancelable" to "async functions are the wrong primitive for cancelable actions".

ljharb commented 9 years ago

@getify how would you propose async functions encapsulate asynchrony in an alternate reality where they didn't return a promise?

getify commented 9 years ago

@ljharb please see earlier in this thread (as well as others) where I already suggested a general way how to deal with that.

FTA: foo(..).pr.then(..) vs foo(..).then(..) and thus foo(..).cancel()

mgol commented 9 years ago

@ljharb Jafar Husain has a proposal for "Composition Functions" where you could use an arbitrary primitive to handle async functions via a similar syntax: https://github.com/jhusain/compositional-functions

Example how final code could look like: https://github.com/jhusain/compositional-functions#refactoring-promise-code-to-use-tasks

RangerMauve commented 9 years ago

I can tell you that right now people are using async/await as it is without any issues. Cancellation is indeed an important thing to consider for async actions, but people are already using es6 promises in the wild, and promises really look like they're here to stay for some time to come. Cancellation can totally be acheived in tandem with using async functions, you just won't have as clean a syntax for it and might need to invent some stuff.

If promise cancellation becomes a part of the spec, then it'd be easy to apply it to async functions as well, until then I think that arguing over some fancy cancellation semantics or making up a new primitive for async values is just standing in the way of getting this out and usable.

getify commented 9 years ago

people are using async/await as it is without any issues

yep, but it's not finalized in the spec yet, so it doesn't have to be permanently set in stone that we made a bad decision (again, IMO) about the return value design.

people are already using es6 promises in the wild, and promises really look like they're here to stay for some time to come

sure. not in contention at all. nothing I suggested does away with (or even changes) them. only changes async functions to return both a promise AND a cancelation trigger, as separate entities.

If promise cancellation becomes a part of the spec, then it'd be easy to apply it to async functions as well

i'm already well on record as thinking cancelable promises is a bad idea. so the argument of, "just wait for this future bad design decision" holds no water for me.

until then I think that arguing ... standing in the way of getting this out and usable.

this is certainly not the first time that we've rushed to get some cool fancy toy out to developers and then circled back later to address "important thing"s and ended up making poor design decisions as a result. but just because we've done that plenty doesn't mean we have to be doomed to repeat that here.

bterlson commented 9 years ago

If we waited for each of everyone's concerns to be addressed before shipping a standard we'd not have a standard. Addressing today's 99% use case with syntax (and if necessary circling back later) seems rational in this environment.

RangerMauve commented 9 years ago

async functions to return both a promise AND a cancelation trigger, as separate entities. I could see that being useful, however as I understand one of the useful things about async functions is that you can seamlessly have something like

async function foo(){
// something
}

async functiion bar(){
  await foo();
}

Wouldn't that have to change to something like

async functiion bar(){
  await foo().promise;
}

Which makes it a lot less clean and less usable, IMO.

Would you perhaps then propose that the returned object has a .then which calls .then on the promise to make the syntax clean again?

How do you make the cancellation nestable?

Would await have to also take the cancelaltion of the async function you're awaiting somewhere? Or does await now take a strange object with a promise and a cancel function instead of a regular promise?

getify commented 9 years ago

If we waited for each of everyone's concerns

cancelation of async tasks is hardly some niche personal concern.

you'll note that this issue was opened almost 10 months ago, long before async functions were stage 3 and gaining widespread usage in the wild. the concerns were raised long enough ago that there was time to address these concerns before holding up the shipping of the feature.

the problem is this thread was largely ignored. scheduling pressure is a poor excuse for not doing the right thing here.

bterlson commented 9 years ago

cancelation of async tasks is hardly some niche personal concern.

Never said it was, nor do I believe that in any sense.

the problem is this thread was largely ignored

I'm sorry you feel that way. In reality, this thread (and many others around GitHub and the web) were discussed repeatedly. You can search the notes for cancellation if you don't believe me, and that's just plenary notes and doesn't take into account the many many conversations that occurred between the champion and interested parties.

scheduling pressure is a poor excuse for not doing the right thing here.

You misstate my argument. Principle of charity would go a long way here.

getify commented 9 years ago

a lot less clean and less usable, IMO

I don't think it's "a lot less" by any stretch. Whether the property is .pr or .promise, it's only a few extra characters.

BTW, considering the significant complications that the other cancelation approaches introduce (like promise cancelation, etc), it doesn't hold any sway to suggest that .pr is going to be too onerous. There's no zero-impact way to address these concerns, but this is by far the least impactful, IMO.

RangerMauve commented 9 years ago

cancelation of async tasks is hardly some niche personal concern.

I agree with that, however async functions aren't about some concept of "async tasks" they're about having syntactic sugar for working with promises. If you want something cancellable, then we need to either have a primitive that supports that, or have promises support it.

don't think it's "a lot less" by any stretch. Whether the property is .pr or .promise, it's only a few extra characters.

It's totally code smell. And it doesn't address the issue of how the cancellation gets nested. The reason async functions return a promise and await takes a promise is that it makes it very easy to compose async functions. If you can't just await the result of an async function, then it means that fundamentally something was overlooked in the design.

Is there anything wrong with making the resulting object a thennable that points to the returned promise? (Other than the fact that the cancellation won't be able to be propagated that way)

getify commented 9 years ago

cancelation of async tasks is hardly some niche personal concern.

Never said it was, nor do I believe that in any sense. You misstate my argument.

That's not my intention. I'm operating in good faith, and assuming the same of others here.

You said:

Addressing today's 99% use case

I interpret that as to say that, in the context of this thread, that it's been deemed that cancelation of async tasks is at most a 1% concern. I strongly disagree with that assertion.

Then you said:

If we waited ... before shipping a standard

Which I interpreted as falling in line with @RangerMauve's assertions that we just need to ship this thing now, as if it's some un-due burden that everyone's expecting this feature and we can't afford any more delay in getting it to them.

If I'm interpreting those impressions wrongly, I'm happy to be corrected.

RangerMauve commented 9 years ago

as if it's some un-due burden that everyone's expecting this feature and we can't afford any more delay in getting it to them.

Well to be fair, browsers are starting to implement it so there is a bit more urgency in having it stable. Not to say that browsers haven't been too hasty in implementing stuff that was eventually withdrawn.

bmeck commented 9 years ago

I want to point out that cancellation means different things.

We could want cancellation to signal stopping and action, we could want cancellation to signal that the result is not necessary (but to continue). I would like to voice my concern that people think some magical cancellation directive will suit all needs. And the tone of some people in this thread is very additive in the case of an error. I feel that when/if Promise cancellation does not fit the need, they will add another extension to Promise to make it suite that need. Promise should not be treated as a piece of data we can staple all behavior to.

Controllers like what are in the Compositional Functions by @jhusain , and controller approach like @getify 's and others have suggested are both more flexible, and feel less like they want to keep adding buckets of bolts to a single data structure.

getify commented 9 years ago

they're about having syntactic sugar for working with promises.

actually, let's be more clear here. they're about syntactic sugar for the generators+promises case. I'm a huge fan of that usage, I teach it all the time, and use it myself in all my code.

but there's a huge difference between writing ES6 generators + promises and writing ES2016'ish async .. await. With generators, I have an external control to cancel/abort the "task" (whatever term you use for it). I actually have this built into my async lib (asynquence), and regularly use it. Generators afford the ability to call .return(..) or .throw(..) on a paused generator. There are many cases where my system design leverages that.

The syntax sugar design of async .. await completely paves over that capability.

The net result is that in those cases I won't be changing from generators + promises to async .. await. That represents the majority of my usage of that pattern.

So we have a design that's supposed to replace the 99% of generators + promises, but at least in my code, it's only going to replace about 5-10% of those usages.

It's tempting to dismiss my concern as the tiny 1%. And if that's the inclination, I suppose there's not much I can do to change that. But I observe that external cancelation forces are extremely common in my coding style, and that I'm not alone in feeling that async .. await falls short of considering that rather important detail.

bterlson commented 9 years ago

@getify quite simply stated, vast amounts of promise-using code written today could be improved with async functions. That is a huge, measurably positive impact on the ecosystem. Standards should not let the perfect be the enemy of the good, and sometimes that means not addressing new use cases (especially extremely contentious ones like cancellation) when we can do something now to address legitimate pain points developers are facing.

getify commented 9 years ago

sometimes that means not addressing new use cases

And what I fundamentally dispute is that cancelation of async is a "new use case".

I am well aware of how powerful the promises+generator (aka async .. await) pattern is in improving promise'd code. That's central to all my teaching on JS and async programming. But cancelation of those tasks is not some rare thing that is only encountered on the corners of applications. It's absolutely central to proper system design, IMO.

I've observed a wide variety of "work-arounds" for external cancelation when working with callbacks, or with promises. I codified those into a "standard" way of dealing with it in the promise+generator pattern. And now async .. await takes a step back and suggests that detail as rather unimportant (1%'er).

RangerMauve commented 9 years ago

Controllers ... are both more flexible, and feel less like they want to keep adding buckets of bolts to a single data structure.

I agree that they're useful, but I don't really see the point in having new syntax for them when you can already easily use function composition with generators for it since it seems like promises aren't necessarily needed for await in its use case and yield already fills the niche for returning an arbitrary value and pausing execution.

getify commented 9 years ago

already easily use function composition with generators

The major argument for async .. await is that the generator+promise pattern presents a burden to coders in that they need a lib to drive the pause/resume cycle. So I think "easily use" is misappropriated here.

If it's "easily use", then we don't need async .. await. If it's a burden, then we need it syntactically to relieve lib burden. But then we shouldn't be so quick to throw out an important function that the lib-driven pattern affords.

getify commented 9 years ago

BTW, it's not clear to me that a future "lib + async .. await" will even be sufficient for handling cancelation, but rather that more than likely it'll be a "lib + generators/promises" answer. As I said, that's going to the majority of my usage, so it feels to me like async .. await is really missing an important piece.

RangerMauve commented 9 years ago

If it's "easily use", then we don't need async .. await.

The controllers/ compositional functions mentioned require external libraries to define the controllers in order to support a wider range of use cases, so I'm not sure I understand where you're going with that.

getify commented 9 years ago

require external libraries

My suggestion of a simple "controller object" with a .pr and cancel() properties on it certainly doesn't require any kind of lib for any developer to use.

If they care about cancelation, they use destructuring to assign off both properties and use each as necessary. If they don't care about cancelation, they simply keep chaining with the extra .pr in the chain, and move on.

but if async .. await ships without a good story for cancelation, hedging its bets on future potential designs that I think are all inferior, the net result is that I (and I expect at least some others) will largely stick with lib + generators + promises. I don't see how that future is considered a win for the design of async .. await.

bterlson commented 9 years ago

@getify It's clearly a new use case since promises are not cancellable. No one even agrees on what cancellation means so to claim there is some well-established "use case" that is cancellation seems fallacious. Yes, in general, people would like to cancel promises and promise returning functions. Yes, it would be great to cancel async functions as well. Addressing that problem will require much effort and IMO now is not the time to do it when we have a proposal that will address (for most people) huge pain points in writing promise code today.

Async functions are not sugar for generators + promises, they are sugar for promise returning functions. The generator is an implementation detail of the async function body.

RangerMauve commented 9 years ago

My suggestion of a simple "controller object" with a .pr and cancel() properties on it certainly doesn't require any kind of lib for any developer to use.

.pr definately clashes with the naming conventions in the rest of the language, so it'd likely need to be .promise

However it still seems like what you want is just something that isn't promises. It's fine to want cancellation, and I genuinely would see use in having cancellable async tasks. However async functions are really about dealing with promises, which don't have cancellation semantics.

If anything it'd be cool if there was a spec specifically for cancellable things like async iterators or observables which has syntactic sugar for working with them. However that would be a different spec with a different primitive at its core.