-
Notifications
You must be signed in to change notification settings - Fork 5
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
Background #1
Comments
I think when.js has cancellation. I'll let @briancavalier explain. The main problem with cancellation that @kriskowal and I have considered is that cancelling provides a communication channel going from the promise to its resolver, representing a capability leak. Before, you could hand out promises to mutually-suspicious parties. With a cancelable promise, each party can affect the other party's operation, which is bad. @wycats and @slightlyoff think of cancellation as a "fourth state," i.e. pending/fulfilled/rejected/cancelled. That is, it's a third path alongside fulfilled and rejected that a promise can take. This doesn't make much sense to me, because it has no synchronous analog. (For example, how would taskjs handle this third state.) But it's a POV that's worth representing. |
I've updated my original post to incorporate some of @domenic's points. |
Cancellation absolutely has a synchronous analog (typed exception handling), it just doesn't have syntax in JS. You wind up building a tiny ad-hoc protocols for it which inevitably wind up being incompatible with other's version of the same thing. This implies that not only do you need it, you need it to be part of any standard you build. |
@slightlyoff for me the killer question is "what would taskjs do with a cancellation state." If there's no good answer to that, it doesn't seem very tenable. On the other hand, I haven't looked much at how Microsoft's In short |
@slightlyoff I was just writing something similar about typed exceptions. @domenic I'm not familiar enough yet with task.js to address what cancellation might mean there. As for when.js, it takes a pretty simple approach and models cancellation as a rejection. So, a cancelable deferred can be created with a cancelation handler that will run (before all other handlers) when the deferred is cancelled (by calling deferred.cancel()), and is allowed to return a cancelation value. That value is then used to reject the deferred's promise. So, from that point on, cancellation behaves exactly like a rejection. That approach was roughly based Dojo Deferred's cancelation mechanism. I have to be honest and say that while it does get some usage, it's fairly limited compared to other features, so it's hard for me to say whether it's "right" or not, but it has worked out for the folks who are using it. I've had requests to also add the cancel() method to the promise, which I've resisted because it seems very wrong: it would allow promise consumers to interfere with one another--it's a rejection, after all. |
I sort of agree with the idea that canceling is like exception handling with similar unwinding the stack considerations for promise chains, but I think one difference is that the user initiating cancel is outside the stack. I'm not sure what the synchronous analog for that is? Thread.interrupt? a late comment now but... Orion Deferred (https://github.com/eclipse/orion.client/blob/master/bundles/org.eclipse.orion.client.core/web/orion/Deferred.js) also supports "cancel" and we have some practical experience with it. We use use it primarily for cancelling long running operations like search and compile tasks and it does work for us. We were supporting "cancelation" primarily to retain functionality our users previously had in Dojo but we were always unsure if the cancel behavior is spec'ed correctly. In particular we found getting the promise chain canceling to work correctly a challenge and although it works for us could do with some guidelines. Also, the considerations around capability leak are also completely valid and a real concern to us. We currently working around this problem by returning promises with empty cancel methods. It might be better if the underlying Deferred could optionally choose to react to the cancel request but that might just be an implementation detail. |
Rather than creating another state, cancelation should be modeled as a subtype of rejection. As @slightlyoff hints, we should specify the type so head the ad hoc protocols off at the pass. My recommendation is that cancelation be modeled as a rejection with an promise.catch(function (error) {
if (error.canceled) {
// ...
} else {
// ...
}
}) On the producer side, a a convenience: resolver.cancel(message_opt) |
My present notion for cancelation is to create a Cancelable from a promise/resolver pair. The Cancelable would look like a Promise and a Promise would look like a Cancelable to a consumer, so the consumer can write code agnostic to whether they have the right to cancel. The Cancelable would have the elevated right to reject. However, it would also have an internal refcount and could be "forked". "fork" would produce a new Cancelable with the right to drop the refcount of the parent cancelable by one and only one when its "cancel" is called. When the refcount drops to zero, it would reject the promise with an Error with a canceled property. Otherwise, the Cancelable would serve as a proxy for the Promise. A general Promise would have noop methods "fork" and "cancel". For convenience, Promises and promise-alikes would have a "cancelable(cancel)" method which would produce a Cancelable proxying the promise and adding a cancelation routine. The This is at least the experiment I’m entertaining for the next rev of Q. It has not proven itself in the wild. |
@kriskowal That seems like a fairly complex approach (3 new methods, plus ref-counting). Do you have use cases in mind that are driving you in that direction? |
C# treats it as a rejection so it's as if you threw an |
As in promises-aplus/progress-spec#1 (comment) I point out that we can use the |
How is taskjs now the gold standard for what we should do? Honestly, it's just doubling-down on the Python/ES6 mistake of exceptions for Generator control flow in the first place...this can't be our mental model for how we proceed. It's nuts. Either you want Promises/Futures as a way for programmers to communicate about operation completion, or you want a concurrency mechanism. It can't be both. Which is it? |
The idea is that consuming promises with Promises are still valuable as a first-class object, so you can e.g. join on them or pass them between layers of abstraction. But forcing code into continuation passing style is a stopgap, and "taskjs" is our shorthand for "the shallow coroutine future." Thus if there are aspects of promises that can't be consumed using shallow coroutines, they become more awkward, forcing you to switch your ES6- and ES7-era code into ES5-style once you start needing those features. "taskjs" is also used by us as a useful reminder that promises are designed as an asynchronous parallel to synchronous control flow, and not as simply event aggregators or "ways to communicate about operation completion." (This is seen in e.g., the return value/fulfillment value and rejection reason/thrown exception parallel, the single-resolution invariant and its corresponding functions-must-return-or-throw-but-not-both invariant, etc. I'm sure we've all read my essay on the subject, so I won't belabor the point.) So again, if there are aspects of promises that are not captured by a synchronous parallel, then we are uneasy with them. And saying "taskjs" is a way of invoking this analogy with the force of a real implementation behind it. This is why e.g. progress invokes such uneasiness, and we are trying here to model cancellation in terms of rejections, since they have a clear synchronous parallel and thus could be consumed in "taskjs" (i.e. in "the shallow coroutine future"). |
This is just not true. At all. With ES6 generators, task.js never needs to use exceptions for control flow. If you're responding to the
That doesn't even make sense. Communicating about operation completion is a concurrency mechanism. Dave |
I think I may have jumped in prematurely and missed some important context (sorry!). Alex, I think you're arguing that cancellation should not be performed via rejection, and the "doubling-down" point is more of an analogy—which I happen to disagree with, but I think it's not the main point. In fact, task.js doesn't currently unify https://github.com/mozilla/task.js/blob/master/lib/task.js#L301 Pretty sure I agree with Alex that cancellation doesn't need to and shouldn't be unified with rejection. But I don't think it's relevant whether you think the Dave |
What are people's thoughts on cancellation propagation? In our Deferred implementation we are currently doing parent propagation for "cancel" and generally have regretted it. What we do is that when 'cancel' is called we will climb the parent chain to find the highest "pending" promise and 'reject' it with a cancellation reason. The reason we did this was to support task code of the style... var task = op1.then(op2).then(op3).then(op4); This might seem sensible but what we've found is that if additional commands have been "then"-ed to op1 we can inadvertently cause them to be rejected as well. An example is if op1 is a common authentication step shared by both a file contents request and search request. If we cancel the promise associated with the search we can inadvertently stop the file content retrieval. This also has the property that things sometimes work based on the timing of the cancel and if op1 has already resolved. I'd propose not doing parent propagation at all. Instead if what is desired is atomic task semantics with parent propagation of cancel, to instead support this in a specialized library. Same goes if you want to do reference counting of children to figure out when to cancel a parent. What are people's thoughts on cancellation propagation to children in the event that the promise has already been resolved or rejected? I personally would hope that the 'cancel' would not propagate to the children since the promise has made it's change but am wondering what others think. I guess I was thinking we could somehow unify 'cancel' and 'reject' so that a call to 'cancel' on the promise would amount to a call to 'reject(CancelError)' but seeing Dave's last post I'm feeling a bit sheepish and wondering what I'm missing. |
@briancavalier With regard to use-cases, @skaegi hints at propagating cancelation. In any substantial program, cancelation must propagate, but how it propagates depends on the situation. I agree with @skaegi; cancelation should not implicitly propagate. var a = getPromiseForA();
var b = a.then(function (a) {
return a + 1;
})
var c = a.then(function (a) {
return a + 2;
});
c.cancel(); In this situation, promises B and C both depend on A. Canceling C does not necessarily mean that promise A should be canceled, but if promises B and C are both canceled, and if no other entities are using promise A, it would be appropriate to cancel A. A more substantial and common problem would be a function that memoizes. var jobs = Map();
function getJobResult(x) {
if (!jobs.has(x)) {
jobs.set(x, startJob(x));
}
return jobs.get(x);
} In this situation, if cancelation were to propagate implicitly, one job user can interfere with another job user by canceling it.
This might be solved by having a convenient mechanism for counting how many users are depending on the job. var jobs = Map();
function getJobResult(x) {
if (!jobs.has(x)) {
jobs.set(x, startJob(x));
}
return jobs.get(x).fork();
} My proposal is that cancelation would be a noop by default and that cancelation might be explicitly hooked up. The most commonly useful way to determine when to propagate cancelation would be counting references to outstanding dependencies. While we’re entertaining use cases, here’s an unrelated matter: var A = getPromiseA();
var C = A.then(function (A) {
return getPromiseB(A);
});
// later
C.cancel(); At various times, assuming nothing is rejected:
|
A really simple alternative to a noop for
With this implementation we get no further cancel propagation other than the regular reject propagation. In the project I work on we are already using rejection for cancelation in our implementation so I don't see anything above that is worse. Providing a cancel handler is currently done when constructing our Deferred but we can instead do this by "then"-ing a reject cancel handler which might actually be slightly better. @kriskowal I have a testcase that looks nearly identical to your last usecase. I need to get that passing before I can commit my change so will report back my implementation of |
Could someone please summarize the reasons for/against "cancel" being a form of rejection? |
Cancellation as a rejectionAdvantages
Disadvantages
Why cancellation can't just be a rejectionThe result of cancellation (i.e. rejection) propagates just like an exception, but the invoking of cancellation needs to propagate the other way. ^ foo()
| .then(() => return foo())
CANCEL .then(() => return foo())
| .then(() => return foo())
v .then(() => return foo())
//vs.
foo()
.then(() => return foo())
REJECT .then(() => return foo())
| .then(() => return foo())
v .then(() => return foo()) If I cancel something, then the result of that cancel propagates up as an exception, but if the promise I cancel was the result of calling then on some other promise, I also want to cancel that other promise, and so on up the chain. The only problem is I don't want to reject all those previous promises, because there might be rejection handlers in that part of the promise chain, and they shouldn't get called, because we've cancelled the operation, and those rejection handlers are intended to handle a different type of error. |
The way I see it there are only two options that are in any way sane for 'what to do when you cancel something'.
I believe what you actually want to do is reject the promise that had cancel called on it initially, then use option 2 to silently cancel all the other promises up the chain (somehow taking account of the issues around forking etc. that @kriskowal talked about) |
Ah I understand now. Thanks. I'll post my opinions once I have studied the problem a bit more. So far the only thing I can say is that cancellation needs rejection or we'll end up having to add yet another callback to every promise to notify the user of the result of an operation. |
So what's wrong with another callback? I can see a strong case that there's some set of APIs that cancel and some that don't. I'm alright with the idea that if you don't provide a cancel callback, we throw a type of CancelError to an error callback. Thoughts? |
Having a Promise.prototype.catchCancelation = function (callback) {
return this.then(null, function (error) {
if (error.canceled) {
return callback(error);
} else {
throw error;
}
});
}; |
I'd tend towards I agree that it would be a very useful extension, but I think that it belongs in the same spec as |
promise.then(onFulfilled, onRejected, onProgress, onCancelled); |
We've changed our Promise implementation so that it has no cancel propagation other than reject and the regular promise assuming for a "then" and all seems to work well and makes a whole lot more sense when debugging. We also have removed the capability of passing a cancel handler into our Deferred constructor and have library code that is similar to We have a few places in our code that require For the use case @kriskowal provided my test case is overriding
e.g. We call the parent cancel and then call the original cancel. This might look overly simple but works correctly as the cancel call is ignored if the underlying promise is already complete. We did try something fancier but it accomplished essentially the same thing and the code was a pain to both read and write. |
So after a discussion with AnneVK today, I think I'm coming around to the MSFT way of doing cancelation: make it a type of rejection, but provide cancel() (and timeout()) methods on the Resolver so that they always result in Error objects with the same well-known properties/values so that they can be easily distinguished in an onreject handler. I've updated the DOMFuture design with this. |
@slightlyoff Your missing the point, propagating that exception and handling that side of things is trivial/a solved problem. Propagating the cancellation is not. var get = require('get-method-that-returns-a-promise-and-supports-cancellation');
function getJSON(url) {
return get(url)
.then(function (res) {
return JSON.parse(res);
});
} It needs to be possible to call |
@ForbesLindesay the security model is what it is. If particular APIs want to vend cancellation capabilities, they need to subclass Resolver and Future to do so (or make the Future type configurable in the Resolver ctor/prototype). And that's an API-by-API contract, not something for the core Future/Resolver API to take on. |
As I've said before specifying cancellation without propagation is worthless. If people want to have their own helper method that rejects a promise with an |
I'm arguing that the I suggest the general case should be |
The problem I see at the moment is that standardizing the error and The other problem is that cancellation is rarely needed. Mostly it's sufficient to just ignore the results. This means that most code will always be written without taking any notice of how cancellation should propagate, so if it doesn't propagate automatically, we'll almost never be able to cancel the underlying operation when we want to. |
I think there is some violent agreement here, in that you've both settled on the minimal piece necessary: cancellation as a specific, standardized type of rejection. @ForbesLindesay is making the point, however, that this is minimal and specifying propagation is (in his mind) necessary for cancellation to be worthwhile. I'm not sure I entirely agree that it's necessary, although it probably would be desirable. |
Considering @ForbesLindesay's example: I think @slightlyoff's point is that // promises returned by `get` have a `cancel` method:
var get = require('get-method-that-returns-a-promise-and-supports-cancellation');
// no cancellation method, no problem with propagation
function getJSON(url) {
return get(url)
.then(function (res) {
return JSON.parse(res);
});
}
// this is what you'd need to do
function getJSONWithCancellation(url) {
var promise = get(url);
var derivedPromise = promise.then(function (res) {
return JSON.parse(res);
});
derivedPromise.cancel = promise.cancel.bind(promise);
return derivedPromise;
} does this make sense to everyone? |
Thanks @domenic, that makes sense. My concern with doing that is that people attempting to write a generic |
I’ve posted my most recent thoughts on cancellation https://github.com/kriskowal/gtor/blob/master/cancelation.md#canceling-asynchronous-tasks |
Interesting ideas. There is one thing about When you know that a promise is used exactly N times (in control flow, not with arbitrary consumers) you would need to call |
Interesting to see different thoughts on cancellation, I'd like to throw my notion on cancellation as well for feedbacks. Specials thanks to @bergus who elaborated #11 for me. My understanding of Promise conceptThe Promise pattern is about Consumer and Producer. ConsumerA request ItemA from the ProducerA, ProducerA creates a Promise pA with TaskA (suppose to produce ItemA) to return to Consumer A immediately, regardless whether ItemA is available now or later. So Promise is the kind of the proxy to access A, providing to Producer utility of What's "cancel"?Thanks to @bergus, led me to this understanding of "cancel": Consumer cancels it's the request (demand of ItemA) from the Producer. Which is only a message to the Producer, so that Producer can response to that, such as:
The main point here is that, it's solely up to the Producer to decide how to react upon cancellation of a request. As stated in the concept section, Cancel handling for the
|
Hi, |
@bergus, initially I also think the same, but after I tried to implement with unit test. I found that having an adding cancelled state to a promise conflicts with the existing usage of Promises. As one of the main benefit for Promise pattern is the handling unexpected result/expection, i.e. rejection, gracefully. However, if cancellation has to wrap itself up, original handling of exception has to extend explicitly to handle cancellation, which introduces rigorous handling needed. IMO that diminish the elegancy of simplistic Promise pattern. And to clarify, what I want to bring up is that, at conceptual level, Promise itself cannot be cancelled, I understand the notion of Producer |
I've opened #15 for further discussion. :) |
Regardless of which approach will be taken, I believe enhancing Promise/A+ with cancellation would be a great addition. Is there a plan to push this thread into reality? |
For the lack of a better venue to bring the topic of cancellation idioms back from the dead, I propose that one good way to deal with cancellation is to take a page from Go’s context object, which deals with deadlines, cancellation, and "context-local-storage". The idiom in Go is for blocking functions to accept a context as their first argument and thread it through their transitive call graph. I've published working code for contexts. The module exports an uncancellable background "root" context and you can create child contexts using The new AbortSignal API for DOM fetch is spiritually similar. |
Cancellation has not yet been put into any of the major JavaScript promise libraries as far as I'm aware.
The only promise library to have any cancellation support is when.js. There has however also been some prior art in the form of C# cancellation tokens. There are a few things to decide upon.
Triggering Cancellation
We need to decide how cancellation is triggered. This is probably as simple as a cancel method on the returned promise. The only problem with doing that is you can't then give a promise (as the result of a function) to multiple mutually suspicious receivers. C# does it with a separate
CancellationTokenSource
object. The relation betweenCancellationToken
andCancellationTokenSource
is analogous to the relationship between promise and resolver.Handling Cancellation
It is easy enough to cancel the resolution of the promise, but there also needs to be a way to handle cancellation of the underlying operation (e.g. downloading a file from a web server). C# provides a
CancellationToken
to the function, which has methods for testing whether cancellation has been requested and for adding an event handler to be triggered when the operation is cancelled.Propagating cancellation
Cancellation should propagate to any underlying asyncronous operations. This process has to be controlled carefully though, because there may be points where you don't want to support cancellation. C# supports cancellation by letting you simply pass on the
CancellationToken
to another asynchronous operation.Correct Behaviour
When a task is cancelled in C# it is rejected with an
OperationCancelledException
. Alternatives would be to simply never resolve the promise, or resolve the promise imediately withundefined
ornull
. It could also be thought of as an additional state (pending/fulfilled/rejected/cancelled)The text was updated successfully, but these errors were encountered: