Open ForbesLindesay opened 11 years ago
I don't understand the difference between uncancellable
and fork
. In both cases you get a promise whom's cancellation does not affect the source promise, and can therefore safely be handed to out to untrusted consumers.
Fork returns a promise which can be cancelled:
var cancellable = getCancellablePromise();
var derivative = cancellable.fork();
derivative.cancel();
derivative
.then(null, function (err) {
Assert.equal(err.name, 'cancel');
console.log('yay, we cancelled it');
});
cancellable
.then(function (err) {
console.log('yay, we didn't propagate the cancel');
});
Uncancellable returns a promise that can't be cancelled at all
var cancellable = getCancellablePromise();
var derivative = cancellable.uncancellable();
derivative.cancel();
// => throws error allong the lines of "Object 'derivative' has no method 'cancel'"
Uncancellable returns a promise that can't be cancelled at all
I assume that behavior propagates through further calls to then
?
var cancellable = getCancellablePromise();
var derivative = cancellable.uncancellable();
var deriv2 = derivative.then(function(x){ return x; });
deriv2.cancel();
Depending on the implementation that may be hard to enforce. So it's good you're leaving it as an extension. I don't really see the need for it though, what do you care if the derived promise you're handing out gets cancelled?
The cancel methd on output must have the same behaviour as for a 'Directly created promise'. The onCancelled method associated with the output promise must have the following behaviour:
Call cancel on the source promise. Call cancel on any child promises.
This definition is problematic. It doesn't actually define expected behavior:
var output = source.then(onFulfilled, onRejected);
output
will be pending if a) source
is pending, or b) child
is pending. Note that there can be only one child
. These scenarios should be listed more explicitly, with prose, rather than pseudo-code. Why even call source.cancel
if output
is pending due to child
?
Furthermore, propagating cancellation is implementation specific, so even saying "Call cancel
on the source promise" seems wrong.
WIP implementation at https://github.com/novemberborn/legendary/tree/cancellation-c with some examples at https://gist.github.com/novemberborn/5282453.
You care because you may want to hand one promise to multiple consumers. You could do this by creating lots of separate forks and handing one to each.
I accept the criticism that it's not the easiest thing to interpret I have zero experience writing specs.
Having the cancellation behavior encapsulated in a subclass is interesting, though it also means you have to take more care in composing promise chains. E.g.:
server.on("request", function(req, res){
var p = app(req);
req.on("close", p.cancel);
p.then(function(response){
res.writeHead(response.status, response.headers);
res.end(response.body.join(""));
});
});
If you can't rely on app()
returning a CancellablePromise
you have to wrap it yourself:
server.on("request", function(req, res){
var p = new CancellablePromise(function(resolve){
resolve(app(req));
});
req.on("close", p.cancel);
p.then(function(response){
res.writeHead(response.status, response.headers);
res.end(response.body.join(""));
});
});
And actually this goes for any promise composition, e.g.:
var c = a().then(function(){
return b();
});
If a()
happens to return an uncancellable promise, calling c.cancel()
won't stop b()
from being called, or cancel its execution.
I suppose this is a trade-off between cancel propagation having unintended side-effects and being super explicit about how to construct a cancellable promise chain? I also suspect libraries that implement CancellablePromise
should use it for composition helpers such as all
, such that users can explicitly opt out of it by calling uncancellable()
.
onCancelled is a function associated with a promise (in an implementation specific manner) that is used to support cancellation propagation as described
Why not standardize on this? It'd make switching libraries easier. One suggestion may be to return the onCancelled
from the resolver
?
My latest implementation attempt is at https://github.com/novemberborn/legendary/compare/cancellation-c-attempt-2, it should support CancellablePromise
, CancellationError
, uncancellable
and fork
.
Yes, you have to be careful because not all promises will support cancellation. This goes for any promise extension though, and is not something special about cancellation. I should add that the cancellation as a separate class is just a tool to explain how it might be implemented. My own "minimal" promise implementation will still include cancellation in core once cancellation is fully specified.
With regards to helpers such as all
. They should propagate cancellation wherever possible.
The reason for not specifying how onCancelled
is attached is two fold:
In short, I think it should be specified, but not until some time in the distant future.
I look forward to looking at the implementation.
I should add that the cancellation as a separate class is just a tool to explain how it might be implemented. My own "minimal" promise implementation will still include cancellation in core once cancellation is fully specified.
Yea, fair enough. I do think I have to go the subclass route if I want uncancellable
to work in Legendary, but maybe legendary.Promise
should be the cancellable one, with another UncancellablePromise
.
Personally I was thinking of just creating a new promise (call it p
) and then doing p.cancel = null
to make it "uncancellable", but that won't be in core.
Took me a while to get everything in working order, but check out https://github.com/novemberborn/legendary/compare/cancellation-c. It's got some tests too!
One tricky edge-case I found is synchronously cancelling a promise before the callbacks have executed, e.g.:
var derived = promise.then(doSomething);
derived.cancel();
IMO doSomething()
should still be executed but its result discarded. It'd be too surprising if sometimes your callbacks don't run.
Nope, the cancellation propagates to promise
and doSomething
is never called, simple.
Sorry, I forgot to add that this is the case where promise
is already fulfilled and thus cannot be cancelled, and the code is aware of it having been fulfilled without being able to observe cancellation as easily.
For directly created promises, let's assume that the cancellation of a promise requires some async work. Wouldn't it make sense then to let onCancelled
optionally return a promise.
var base = Promise(function(resolve, reject){...}, function(){
return new Promise(...); // resolved when cancellation is finished
});
So the addition to the 3 rules for when calling cancel
on a pending
promise would then be something like:
onCancelled
returns a promise the base promise is rejected with CancellationError
when the returned promise resolves. If the returned promise is rejected with error err the base promise gets rejected with err.
Based on discussions in #6
Terminology
In addition to the terminology from Promises/A+ we use the following:
CancellationError
is an error used to reject cancelled promises.onCancelled
is a function associated with a promise (in an implementation specific manner) that is used to support cancellation propagation as described.Requirements
Promises can be created in two ways:
Directly created promise
These are promises that are created directly using the API for creating a new promise exposed by the library.
Cancellable promises have an additional
onCancelled
function provided by the creator of the promise. This can be a no-op, but should typically be used to cancel the underlying operation (e.g. an ajax request).Directly created promises have a cancel method.
When
cancel
is called on apending
promise it:onCancelled
with no argumentsonCancelled
throws an error it rejects the promise with that error.If
onCancelled
does not throw an error it rejects the promise with aCancellationError
Promise created by
then
When
then
is called, it creates a new promise. Call this theoutput
promise. Call the promise thatthen
was called on thesource
promise. Call any promise returned by a callback or errorback achild
promise:If
source
is a cancellable promise thenoutput
must be a cancellable promise. Thecancel
methd onoutput
must have the same behaviour as for a 'Directly created promise'. TheonCancelled
method associated with theoutput
promise must have the following behaviour:cancel
on thesource
promise.Call
cancel
on anychild
promises.The
CancellationError
When a promise is directly cancelled it is rejected with a
CancellationError
error. A CancellationError must obey the following criteria:cancellationError instanceof Error === true
).It must have a
name
property with value"cancel"
.Recommended Extensions
The following two extensions may be provided, and if provided should behave as follows:
Uncancellable
unacnellable
should return a new promise which will be fulfilled or rejected in the same way as the current promise but does not have acancel
method or has a no-op in place ofcancel
.Fork
fork
should return a new cancellable promise (output
) which will be fulfilled or rejected in the same way as the current promise (source
) but where it won't cancel thesource
if theoutput
promise is cancelled.Sample Implementation
An example implementation may make the behavior described above easier to follow.
The following sample extends
Promise
to produceCancellablePromise
and uses ES6 syntax: