petkaantonov / bluebird

:bird: :zap: Bluebird is a full featured promise library with unmatched performance.
http://bluebirdjs.com
MIT License
20.45k stars 2.34k forks source link

3.0 cancellation overhaul #415

Closed petkaantonov closed 8 years ago

petkaantonov commented 9 years ago

The current cancellation seems to be the most problematic feature due to some annoying limits in the design:

Since all consumers and producers must be trusted/"your code", there is no reason to enforce that cancellable promises are single-consumer or create new primitives.

Edit: The below design has been scratched, see https://github.com/petkaantonov/bluebird/issues/415#issuecomment-73242358

In the new cancellation you would register the callback while marking the promise cancellable:

promise.cancellable(function onCancel(reason) {

});

This returns a new cancellable promise (unlike right now which just mutates existing promise which causes a lot of headache internally) and the flag will automatically propagate to all derived promises. However, the reference to the onCancel callback will only be held by the new promise created by .cancellable(). Calling .cancel() on a cancellable promise will keep propagating to cancellable parents and calling their onCancel callbacks.

The onCancel callback will receive the cancellation reason as its only argument (the default is a CancellationError instance).

From the callback it's possible to decide the fate of the promise:

(However I guess 99.99% of the time you want throw reason, so this flexibility adds a lot of inconvenience?)

Bad (should use .timeout() for this) example of usage:

function delay(value, time) {
    var timerId;
    return new Promise(function(resolve, reject) {
        timerId = setTimeout(function() {
            resolve(value);
        }, time || value);
    }).cancellable(function(reason) {
        clearTimeout(timerId);
        throw reason;
    });
}

var after500ms = delay(500).catch(CancellationError, function(e) {
    console.log("Your request took too long");
});
delay(250).then(function() {
    after500ms.cancel();
});

Artazor commented 9 years ago

@petkaantonov ok. Since the proposed semantics is considered to be a breaking change, can we model the proposed semantics by the following means (for a while):

Can this approach be viewed as an approximation/transition (even maybe on the "monkey-patch" level)? I'm starting a new big project and feel that the forget semantics is correct and want to use it early (to be ready when it will standardized)

The only thing I'm afraid of that the proposed sematics does not compose well with the other promise implementations. Especially I'm consdering the "adoption of the state" when somebody returns ForgetablePromise as the result of the onFulfilled/onRejected inside then that doesn't belong to the bluebird.

petkaantonov commented 9 years ago

(to be ready when it will standardized)

What do you mean by this? Cancellation is theoretically extremely incorrect - it will never be standardized.

Artazor commented 9 years ago

I mean the (in)famous 2.3.2 for interoperability when/if "forget" semantics will be implemented by the several implementers (and I hope it will).

petkaantonov commented 9 years ago

That's not what the Promises/A+ is about, it is only about interop not features, it already hampers innovation enough already as it is. You should not mix promise implementations in your codebase anyway, but convert a promise from an API as soon as you get it (same as how you always promisify at lowest level so at no point you are using a callback in your code at all). That way interop is not a problem even with cancellation.

Artazor commented 9 years ago

@petkaantonov ok, it's clear.

  1. Have you any thoughts about when/if the 'cancel-as-forget' semantics will be available in Bluebird?
  2. I've noticed that the decision was postponed from the version 3.0.0 to the 4.0.0 one. However, are you sure that implementing this later will cause less pain than implementing it immediately?
  3. What are you thinking about the following marking promise as cancellable:
function cancellableDelay(ms) {
   return new Promise(function(resolve, reject) {
        var handle = setTimeout(resolve, ms);
        return function() {
             clearTimeout(handle);
        }
   });
}

i.e. simply returning the cancellator from the resolver function marks the promise as cancellable. Here cancellator is allowed to return a Promise/Thenable, so the cancelling will be propagated to the downstream only after fulfillment of that promise (since the .cancel() was called).

I suppose it is a good way to incapsulate the cancellation logic. Isn't it? (Or you are completely against using closures due to a performance hit?)

Maybe this approach also will allow to implement both semantics simultaneously:

Will it yield to the chaos?

petkaantonov commented 9 years ago

The tag was made 4.0.0 so that it wouldn't block 3.0.0 in case there was no good solution to resource cleanup. It will come with 3.0.0, however I am currently extremely busy with something else right now.

As you say, marking specific promises as cancellable and uncancellable at runtime is extremely problematic so all promises are always cancellable when the cancellation feature is enabled (it is by default disabled and can only be enabled before creating any promises).

The old cancellation semantics is not possible, however you could simply reject a promise with a cancellation error (the error type is not removed).

The cancellation handler can be registered from outside with onCancel, (see branch 3.0 cancel tests) new Promise is almost always an anti-pattern.

Artazor commented 9 years ago

@petkaantonov, of course every new Promise resembles the Deferred antipatern.

I meant the case when you have a non-promisified external api with an explicit ability to abort the workflow (kill the process). The example above only demonstrates a hypothetic approach to promisify such an api (only for example) to embed a cancellation directly into promise creation.

I simply have a keen sense that if there is a create/kill pair for some process, then these methods deserve to be incapsulated as much closer as possible. Without ever creating an intermediate promise. On the other hand an appropriate task scheduler will guarantee that the intermediate promise will never have a chance to break anything down. But still...

Am I right, that the right way to promisify setTimeout/clearTimeout [as an example] according to 3.0.0 will look like this:

function delay(ms) {
    var handle;
    return new Promise(function(resolve){
        handle = setTimeout(resolve, ms);
    }).onCancel(function(){
       clearTimeout(handle);
    });
}

?

benjamingr commented 9 years ago

@petkaantonov, of course every new Promise resembles the Deferred antipattern

No it doesn't. Using new Promise has plenty of use cases - the deferred anti-pattern is about wrapping an existing promise returning function with new Promise

petkaantonov commented 9 years ago

@benjamingr there is only one use case, to wrap an async api?

petkaantonov commented 9 years ago

@Artazor yes, and it doesn't create new promise it attached the listener on existing promise

Artazor commented 9 years ago

As long as I see (while inspecting /test/mocha/cancel.js of the 3.0.0 branch) cancellation still can be caught via catch-all .caught(), and the didReject function passed to .then(). Thus the cancellation is still a rejection.

To avoid accidental handling of a cancellation can we use the following three simple rules:

  1. Avoid using didReject parameter of .then(), and let it only for the promise composability;
  2. Use .caught() only for known types of errors by filtering them with either Error classes or predicates [non-standard yet wanna-be-a-standard feature];
  3. Ignore unhandled rejections if the reason is CancellationError.

(?)

When/if the 'cancel-as-forget' semantics will be implemented then all those restrictions will disappear.

[offtopic: sorry about asking here, but what is the status of the https://github.com/sparhami/bluebirdBreaksAsyncStackTraces - is the Bluebird chrome-async-stack-frame-friendly? if not - maybe it worth to introduce some flag that prevents grouping of unrelated promises for debugging purposes?]

Artazor commented 9 years ago

And still I feel the huge difference between two distinct use cases of the proposed .onCancel()

  1. What you should do to cancel the pending execution
  2. What you should do when cancellation has happened

Arrrrghhh....

bergus commented 9 years ago

@petkaantonov What do you think about this draft (initial idea at promises-aplus/cancellation-spec#11)? I think it matches the semantics that you proposed above very well.

petkaantonov commented 9 years ago

[offtopic: sorry about asking here, but what is the status of the https://github.com/sparhami/bluebirdBreaksAsyncStackTraces - is the Bluebird chrome-async-stack-frame-friendly? if not - maybe it worth to introduce some flag that prevents grouping of unrelated promises for debugging purposes?]

I was unaware of such an issue but as far as I can see it could be easily solved by skipping the trampoline when running under debug mode.

petkaantonov commented 9 years ago

@bergus I am thinking if branched promises are common enough and if it is truly bad if one branch can cancel the others to warrant such implementation complexity, although to user the api seems simple enough with no need to carry around separate token objects.

bergus commented 9 years ago

@petkaantonov I invented that "token registration" just because the main concern about cancellable promises (instead of a separate, explicit cancellation mechanism) was the multiple consumer case. There were other proposals that suggested to use explicit .fork() calls, but I didn't want to put that burden on the user. I did hope that the implementation would not be too complex, and could be optimised for the the common single-consumer-only case. I would be happy as well if you implemented "one cancellation attempt cancels all branches" and we see that it doesn't matter in the real world (judging from user feedback). So go for it :-)

I noticed another, more important difference between our proposals though: You are talking of forever-pending promises. I think our goal is the same, we don't want to execute all the callbacks (and exception handlers) that led to the cancelled promise, there is no "root rejection" that bubbles through the chain; only finally-handlers would be invoked. However, I think we need to reject all the cancelled promises nonetheless (without calling the handlers in the cancelled chain), to care for other branches (that would otherwise be never notified).

var p1 = somePromise() // will get cancelled below
         .then(function() { /* should not be called */ })
         .catch(function() { /* should not be called */ })
         .finally(function() { /* should still be called */ });
var px = p1 // branch
         .then(function() { /* should not be called */ },
               function() { /* should be called with a CancellationError */ });
var p2 = p1 // chain
         .then(function() { /* should not be called */ });
var p3 = p2 // continue chain
         .then(function() { /* should not be called */ },
               function() { /* should be called with a CancellationError */ });

p2.cancel(); // mid-chain cancellation

// later
p2.catch(function() { /* must be called with a CancellationError */ });
Artazor commented 9 years ago

Another confusing moment is an interoperability with generators. Promise.coroutine should be aware of cancellation of the promises yielded by the generator. The only possible reaction for the cancellation I can imagine - is to throw a CancellationError into the generator. In the same way, CancellationError caught from gen.next() should cancel the resulting Promise. On the other hand, catch-all clauses in the generator's code can compromise the 'forget' semantics (as we don't have real uncatchable exceptions).

petkaantonov commented 9 years ago

Generators have .return() method which matches cancellation semantics exactly (active .finally is called and then it returns immediately)

petkaantonov commented 9 years ago

Although it's unimplemented atm, but it's in the spec

Artazor commented 9 years ago

Hmm... I was aware of G.[[Close]] but it seemed to me that it has been removed from v8. But it's fortunately back: G.[[Close]] has been reborn as the generator.prototype.return in the latest unofficial ES6 draft, so we have chances to obtain a consistent cancellation even within coroutines.

Does it mean that we should postpone the entire implementation of the forget semantics until generator.prototype.return is implemented?

Btw, there is a discussion at the esdiscuss where @rbuckton proposes alternative solutions. However, I think that the forget semantics implemented using generator.prototype.return has the greatest "profit/unsoundness" ratio.

Maybe it worth to join both discussions? @rbuckton, @petkaantonov, @ForbesLindesay ?

Artazor commented 9 years ago

@petkaantonov since onCancel()/cancel() in the proposed design are synchronous what do you recommend for the cases when cancellation logic itself involves Promises (i.e. is asynchronous)?

petkaantonov commented 9 years ago

@Artazor what do you mean by cancellation logic? the only logic you can put in onCancel is resource cleanup

bergus commented 9 years ago

@Artazor the cancellation logic can do whatever it wants (do nothing, take forever, etc), .cancel() does not wait for it and does not return anything either. If you want to signal any progress from a cancelled promise, you have to explicitly use an extra channel.

Artazor commented 9 years ago

I've tried to model this channel through the finally + onCancel

Promise.prototype.whenCancel = function(fn) {
    var cancelled = false;
    return this.onCancel(function(){
        cancelled = true;
    }).finally(function(){
        if (cancelled) return fn.call(this);
    });
}

Promise.prototype.cancelThen = function(didCancel) {
    var self = this;
    return new Promise(function(resolve){
        self.finally(resolve).cancel();
    }).then(didCancel);
}

And apply it as follows:

var p = new Promise(function(){}),
    resource = {
        stop: function() { 
            console.log("Stop is synchronous"); 
        },
        cleanup: function() { 
            return Promise
                .delay(1000,"but cleanup isn't")
                .then(function(x){
                    console.log(x);
                }); 
        }
    };

p.whenCancel(function(){
    console.log("Aborting...")
    resource.stop();
    return resource.cleanup().tap(function(){
        console.log("Cleanup performed")
    });
}).cancelThen(function(){
    console.log("All is cancelled");
})

But it looks like that on cancel all finally handlers are triggered at the same time, and the order of the finally handlers is lost if some of them return promises.

UPD: (my fault), it works, finally order is preserved even on cancel as expected (even if there are promises), so it can be used when needed.

Artazor commented 9 years ago

@bergus I understand

zowers commented 9 years ago

Note there's ongoing discussion about aborting a fetch() https://github.com/whatwg/fetch/issues/27 probably both bluebird and fetch could benefit of joining forces

WebReflection commented 9 years ago

Trying to summarize here what happened there (following the "already scratched" list on top)

There is an implementatio playground that works as described in above points.

Following few examples.

Where to define cancel-ability?

// will resolve in a second, not cancelable
var regular = new Promise(function (res, rej, howToCancel) {
  var timer = setTimeout(res, 1000, 'OK');
});

regular.cancel(); // will throw

// will be a cancel-able Promise
var cancelable = new Promise(function (res, rej, howToCancel) {
  var timer = setTimeout(res, 1000, 'OK');
  // define cancel-ability
  howToCancel(function () {
    clearTimeout(timer);
    // note: users don't have to reject
    // it's done implicitly on `.cancel()`
  });
});

// will cancel and set state as rejected
cancelable.cancel();

// it is possible to eventually send a reason
// cancelable.cancel({reason: 'because'});

Where does cancel ends up in a chain?

new Promise(function ($res, $rej, ifCanceled) {
  var internal = setTimeout($rej, 1000);
  ifCanceled(function () {
    clearTimeout(internal);
  });
})
.then(
  function () {
    console.log('on time');
  },
  function () {
    // we'll end up/stop the chain here
    console.log('error');
  }
)
.catch(function () {
  console.log('caught'); // won't happen
})
.cancel({because:'reason'})
// this will never be executed
// but it will be resolved anyway
.then(function (value) {
  console.log(value);
});

// or ...
new Promise(function ($res, $rej, ifCanceled) {
  var internal = setTimeout($rej, 1000);
  ifCanceled(function () {
    clearTimeout(internal);
  });
})
.then(
  function () {
    console.log('on time');
  }
)
.catch(function () { // we'll stop here
  console.log('caught');
})
.cancel({because:'reason'})
.then(function (value) {
  console.log(value);
});

How to own cancel-ability and pass a non cancel-able Promise?

// my private cancelable Promise
var cancelable = new Promise(function (s, j, c) {
  c(clearTimeout.bind(this, setTimeout(s, 1000, 'OK')));
});

// the one I'll expose
var nope = new Promise(function (s, j) {
  cancelable.then(s, j);
});

Goals tried to achieve with this proposal:

Things to be defined:

rbuckton commented 9 years ago

@WebReflection I understand your concern that having a separate cancellation token is ugly/dumb, however I think that the approach has the following benefits:

I put together a summary of various examples of cancellation using both regular functions and Promises, as well as with async functions and await to illustrate the value.

I do like the idea of having an additional method on the Promise prototype for observing the cancellation of a Promise, without explicitly rejecting the Promise on cancellation. This is analogous to .NET Tasks and cooperative cancellation, where you can register a task continuation that is only triggered on cancellation. With the token approach, that would mean adding the ability to pass a token as an additional argument to the Promise constructor, or the then or catch methods on the Promise prototype so that the Promise could register an internal cancellation mechanism with the token. This could then be observable on the Promise instance by listening to some kind of canceled method:

function exec(token) {
  return new Promise(resolve => { ... }, token);
}

let source = new CancellationTokenSource();
exec(source.token)
  .then(x => { ... })
  .canceled(() => { ... });

// cancel the operation
source.cancel();
WebReflection commented 9 years ago

@rbuckton answering about my POV on your benefits ...

Last, if you do like the idea of having an additional method on the Promise prototype for observing the cancellation then all this decoupling you talked about would be even more messed up 'cause you might realize it is cancelable, and nobody gave you that power, holding who knows where, and in how many, that "big red button"

My 2 cents

ProTip commented 9 years ago

I spent quite a while looking into this the other day and as somebody new to coffeescript/nodejs but with a lot of C# experience I'm pretty dead-set on have a good async workflow.

Unfortunately when it came time to use cancelling to stop a coroutine I fell into a quagmire. I need to be able to treat async functions(coroutines) as promises so I can yield them or bundle them up as required but for the life of me couldn't figure out any cancellation support on them.

For what it's worth, I found somebodies cancellation(tokens) package and it has worked fine for me; I'm off to the races and dev moves forward.

fresheneesz commented 9 years ago

I didn't see this issue, and wrote this issue: https://github.com/petkaantonov/bluebird/issues/663#issuecomment-112997211

@WebReflection What happens if cancellation occurs after a couple thens in the chain have already been called? Like this:

var yourProcess = new Promise(function ($res, $rej, ifCanceled) {
  var internal = setTimeout($rej, 1000);
  ifCanceled(function () {
    clearTimeout(internal);
  });
})
.then(function () {
    console.log('on time');
    yourProcess.cancel({because:'reason'})
})

yourProcess.catch(function () { // does it stop here? I feel like it should
    console.log('did i catch it?');
    //yourProcess.cancel({because:'reason2'}) // what if cancellation happens here instead?? Is it too late?
})
.then(function (value) {
  console.log(value);
});

I think for cancellation to be appropriately powerful, you need to somehow define all the individual promises that should be cancelled - identifying just one promise and then propogating that to all ancestors or all descendants doesn't cut it. The easiest way to define a list of all the individual promises you're likely to mean is to define a range - say that all promises between A and B are cancelled.

If the way you do this is to define a new Promise chain and call cancellable on it, I think cancellation ranges could be pretty easy to define. Example:

var  A = new Promise(function(ra,ta,ifCanceledA) {
  ifCanceledA(function() {
    console.log("A cancelled")
  })

  var B = new Promise(function(rb,tb,ifCanceledB){
    // event 1
    ifCanceledB(function() {
      console.log("B cancelled")
    })
  }).then(function() {
     console.log("B done")     // event 2
  }).cancellable()

  return B.catch(function(e) {
    return "B's cancellation caught" // event 3
  })
}).then(function() {
   console.log("A done")     // event 4
}).cancellable()

There are a few different important scenarios: I. If A is cancelled anytime before event 4 happens, "A cancelled" is printed, and A is rejected with a CancellationException II. If A is cancelled after event 4, "A done" prints, and A is rejected with a CancellationException III. If B is cancelled anytime before event 1 happens, "B cancelled" is printed, then "A done" is printed, and A resolves to "B's cancellation caught". IV. If B is cancelled after event 1 and before event 2, "A done" is printed, and A resolves to "B's cancellation caught". V. If B is cancelled after event 2, same thing happens as in step 4 except "B done" is printed first

So in steps III, IV, and V, A isn't cancelled because cancelling B strictly defines the callbacks that can be cut off as the ones that make events 1 and 2 happen. This way you can define and pass around arbitrarily specific chains that can be cancelled, even if they're nested inside other changes, or called in two different chains, without affecting promises outside that defined range.

I think this is simpler than a cancellation token and yet just as powerful (if not more so).

Thoughts?

bergus commented 9 years ago

@fresheneesz I think you'll want to read #565. Things like not propagating cancellation if there are other callbacks waiting for a promise are already implemented :-)

fresheneesz commented 9 years ago

@bergus What part of my post are you addressing? My proposal there covers a lot more than just that.

WebReflection commented 9 years ago

@fresheneesz ...

I think for cancellation to be appropriately powerful, you need to somehow define all the individual promises that should be cancelled

cancellation should be possible at any time for chainability reason which is part of the strength of Promises ( .then().then().then() ) so if it wasn't canceled already, it should cancel the very first encountered cancelable promise in the chain.

If already canceled, nothing happens, you keep going as you would have done in any case. This is inevitable unless you want to expose canceled as property and behave accordingly but I think that's superfluous.

Since it's the author of the Promise to decide its cancelability, whoever want to return a cancelable promise in the chain can simply do it, canceling the external promise when its new one with its new cancelability is invoked.

var  A = new Promise(function(ra,ta,ifCanceled) {
  ifCanceled(function() {
    console.log("A cancelled")
  });

var B = new Promise(function(ra,ta,ifCanceled) {
  ifCanceled(function() {
    A.cancel();
    console.log("B cancelled")
  });

at that point you can simply pass B around ... I think this a very simplified solution but for all use cases I could think of it should just works.

fresheneesz commented 9 years ago

@WebReflection

if it wasn't canceled already, it should cancel the very first encountered cancelable promise in the chain

Do you mean the very first cancelable promise in the chain's ancestry, or do you mean its descendancy? It would be much clearer for me personally if you could address the specific example I brought up.

whoever want to return a cancelable promise in the chain can simply do it, canceling the external promise when its new one with its new cancelability is invoked.

And how do you program something if you want to only cancel that internal promise, and not the external promise? This is what my proposal addresses.

WebReflection commented 9 years ago

sorry, ancestry of course, you can decide to pass around a cancelable Promise you should never be able to cancel Promises you don't own or didn't receive ... so ancestry or nothing.

And how do you program something if you want to only cancel that internal promise, and not the external promise? This is what my proposal addresses.

You wrap it through a cancelable Promise as my example does ... it passes around B that once canceled can cancel A too. Whoever receives a Promise, receives B, and will be unable to directly cancel A.

fresheneesz commented 9 years ago

ancestry or nothing

Makes sense

it passes around B that once canceled can cancel A too

In the last example you posted, A and B aren't related in any way except that if B is cancelled A is cancelled. I'm much more concerned with promise chains, not individual promises.

I realize I'm joining this discussion late, and I don't want you to repeat everything you've already said just so I can understand, but you mentioned you wrote "3 proposals", which I assume are API proposals, and I can't find them either in the esdiscuss.org link that petka gave, or in this issue comment thread. Is there a current work-in-progress proposal we are discussing?

Also, do you understand the proposal I put forth? What are the shortcomings you see in it?

fresheneesz commented 9 years ago

One thing I just thought about, if you have some conceptual processes X, A, and B, where A and B are parts of X, like this:

X {
  A {
    // ...
  }
  B {
    // ...
  }
}

Cancelling X should cancel all of process X, including processes A and B. But if what a cancellation does is create an exception that propogates, something inside A might catch and "handle" that cancellation, so that part of process A and all of process B actually continues. This isn't what you would want in a cancellation right? It looks like some people in this thread have balked at having a third state ("cancelled"), since it wouldn't match spec, but that seems like the cleanest way to handle it. You don't want some unknown inner process catching the CancellationException and overturning the cancellation. How else would you get around this without having a 3rd state - the cancellation state?

WebReflection commented 9 years ago

@fresheneesz you cancel what you want to cancel and what you have access to or what you create as cancelable. You should really think it at that simple logic, and no magic involved.

https://github.com/whatwg/fetch/issues/27#issuecomment-87617663 https://github.com/whatwg/fetch/issues/27#issuecomment-87634930 https://github.com/whatwg/fetch/issues/27#issuecomment-87695054 https://github.com/whatwg/fetch/issues/27#issuecomment-87882267 https://github.com/whatwg/fetch/issues/27#issuecomment-87884170

but you should really probably take your time and read the entire thing there and not just my opinion or code examples.

Anyway, I'm really done here because there's nothing more I need to add or to understand and mostly everything has been told already.

I'm every day more convinced Promises are just the wrong solution for anything asynchronous that might need to be dropped without needing to throw an error.

Promises are great for small/quick things, like pressButton.then(switchLightOn);, where you won't even have time to change your mind or it's just cheap to pres the button again and switch it off.

Promises are also great for server side tasks where you want that your full chain of asynchronous actions is executed from start to end without problems ... no interference there meant, wanted, or needed.

However, if you use Promises for chains that involves unpredictable amount of time to execute you'll realize that patterns like pressButton.then(closeDoors).then(reachFloor).then(openDoors) will chop people that will pass in the middle while the doors were closing; unable to simply re-open and close again after, without needing to throw and trash the entire chain with an error.

It's not an error, it's just a little change to the very same initial action of reaching the floor, user shouldn't need to start the action again or be notified with an alarm that an error occurred.

We should never forget the importance of the time variable and we also don't have crystal balls.

If you need to change an action that is taking time to execute you, as developer, should be able to do that. This is how pragmatic I believe this matter is, this is how .abort() worked so far and pretty well, so you could react through a listener or just ignore it: you were in control.

Internally, indeed, every Promise can be somehow "aborted", but I guess we just like playing philosophy on user-land and we are also often those very same that create problems to ourselves ^_^

Well, at least it's never boring here, but I'm a bit tired of this topic and I simply avoid Promises whenever it makes sense doing it.

I do hope this entire story taught us at least that using Promises for everything asynchronous is a bloody footgun and that events still have many valid uses cases and applications that should probably never be replaced with Promises .... or let developers wrap them when it's convenient, so that everybody wins, and everyone can control "the flow".

Best Regards, please contact me privately if you'd like to discuss more. I'm off this thread now.

spion commented 9 years ago

@WebReflection I think that BB 3.0 cancellation semantics may change your mind once they're released. We'll see...

bergus commented 9 years ago

@fresheneesz

It looks like some people in this thread have balked at having a third state ("cancelled"), since it wouldn't match spec, but that seems like the cleanest way to handle it. You don't want some unknown inner process catching the CancellationException and overturning the cancellation. How else would you get around this without having a 3rd state - the cancellation state?

There are two points:

There is no such thing as "overturning" because the descendant chain is already cancelled.

spion commented 9 years ago

@bergus except finally callbacks to run the cleanups. I disliked it at first, but after reading all the arguments I think I'm sold! :)

fresheneesz commented 9 years ago

@WebReflection Thanks for the list of examples, that helps a lot! I definitely agree that events have a completely different use case than promises, and any case where you have an event happening multiple times, promises don't help you there.

In any case, I think I understand a lot more about what you said about it being more simple.

@bergus Yeah, i was actually thinking of a case where you define different cancelable processes in the same chain like you wrote up on march 16th, like A.then(B).then(C) and you want to have the choice of only cancelling B and also cancelling A, B, and C. I see now this just isn't the right way to define separate processes, and you should instead do something like: A.then(function(){return B}).then(C) in which case its very clear what should happen if you cancel B - A is left intact if B catches its cancellation, but A gets a cancellation error otherwise. So I feel a little more ok about not having a third state.

But cancellation is complicated, and there are a couple cases I want to bring up:

  1. For var x = A.then(function(){return B}).then(C), if B is cancelable, and x is cancelled during process B, process B should also be cancelled
  2. UNLESS B is being used by some other process that hasn't been cancelled, in which case, it should only be cancelled if all processes its being used by are cancelled,
  3. EXCEPT when B is still needed by a future process, and we want it to keep running so as to keep latency down. In this case, if we followed the ideas in 1 & 2, we would need some explicit way to mark B as not cancellable by its users. Something like var B = processB.selfCancellable(), defining it as only cancellable if B.cancel() is called, and not when processes using it are called. Alternatively, we could go the other way around, and require a special marking for processes that can be cancelled by processes using it. Something like processB.parentCancellable().
  4. For var x = A.then(function(){return B}).then(C), if B is cancelled, there should be some way to define x as dependent on B, and cancel x when B is cancelled. This one should be easy, just B.catch(Cancellation, function() {x.cancel()})
  5. Cancellability should be easily definable for a whole promise chain, without having to explicitly define something for each part of the chain, so that if your chain has 10 steps, you can cancel on step 5, and only the cancellation callbacks (ifCancelled) for steps 6-10 will be called.
  6. If you do cancel a chain of 10 steps, the error should be caught by a step later than 10 regardless of there being any catches on steps 10 or less. I'm unsure whether or not catches on steps 10 or less should be called.
  7. If you have a branching continuation like: var x = A.then(B); var y = A.then(C), if x is cancelled, B should be cancelled, but A should not be cancelled, for the same reason as in case 2, with the same caveat as in case 3

Are these cases kind of in line with how bluebird's v3 cancellation is currently being designed?

bergus commented 9 years ago

@fresheneesz Thanks for your input, yes cancellation is complicated :) I'll try to address all of your points:

For var x = A.then(function(){return B}).then(C), if B is cancelable, and x is cancelled during process B, process B should also be cancelled

It is - supposed that A is fulfilled and B has not already settled.

UNLESS B is being used by some other process that hasn't been cancelled, in which case, it should only be cancelled if all processes its being used by are cancelled

Yes, if B is used elsewhere (and has other uncancelled callbacks attached to it), it won't be cancelled. Only the callback that resolves the A.then(() => B) promise will be "forgotten" about.

EXCEPT when B is still needed by a future process, and we want it to keep running so as to keep latency down. In this case, if we followed the ideas in 1 & 2, we would need some explicit way to mark B as not cancellable by its users. Something like var B = processB.selfCancellable(), defining it as only cancellable if B.cancel() is called, and not when processes using it are called. Alternatively, we could go the other way around, and require a special marking for processes that can be cancelled by processes using it. Something like processB.parentCancellable().

Not as complicated as this. You just would do var bForFuture = B.then(); and B is not cancelled by our process until bForFuture is as well.

For var x = A.then(function(){return B}).then(C), if B is cancelled, there should be some way to define x as dependent on B, and cancel x when B is cancelled.

x is automatically dependent on B as soon as A fulfills and the callback returns B. At that point, B can't be cancelled until x is cancelled. If B is already cancelled by the time A fulfills, then x is rejected with the cancellation error.

Cancellability should be easily definable for a whole promise chain, without having to explicitly define something for each part of the chain,

Indeed it is. then does create a new promise that is dependent on its ancestors and will propagate cancellation automatically, no explicit syntax required.

so that if your chain has 10 steps, you can cancel on step 5, and only the cancellation callbacks ( ifCancelled ) for steps 6-10 will be called.

Not sure what you mean. If there are still the steps 6-10 depending on the promise 5, you cannot simply cancel that. You have to cancel promise 10 first.

If you do cancel a chain of 10 steps, the error should be caught by a step later than 10 regardless of there being any catches on steps 10 or less. I'm unsure whether or not catches on steps 10 or less should be called.

Again, there is no error to be caught when you cancel anything. The only error you will receive is the one when you then on an already cancelled promise.

If you have a branching continuation like: var x = A.then(B); var y = A.then(C), if x is cancelled, B should be cancelled, but A should not be cancelled, for the same reason as in case 2, with the same caveat as in case 3

Yes, A isn't cancelled, it's still needed for y. Unless you "forcibly" cancel A (not sure whether or how that may be possible), but in that case y would be rejected.

fresheneesz commented 9 years ago

@bergus Thanks for the responses!

If there are still the steps 6-10 depending on the promise 5, you cannot simply cancel that. You have to cancel promise 10 first.

Yes I meant "you can cancel [the promise for step 10] on step 5, and only the cancellation callbacks .. for steps 6-10 will be called."

Again, there is no error to be caught when you cancel anything. The only error you will receive is the one when you then on an already cancelled promise.

So lets say you have var x = A.then(B).then(C). If you cancel x inside B, what happens to B? Does it have some kind of ifCancelled that will be called (if its defined)?

In any case, sounds like 3.0 cancellation will be pretty great! Would love to see the docs for the proposed/WIP 3.0 cancellation API.

bergus commented 9 years ago

@fresheneesz

you can cancel [the promise for step 10] on step 5, and only the cancellation callbacks .. for steps 6-10 will be called.

Yes indeed, if the promises up to step 5 are already settled, then they are no more cancelled.

So lets say you have var x = A.then(B).then(C). If you cancel x inside B, what happens to B? Does it have some kind of ifCancelled that will be called (if its defined)?

If we have x = a.then(function B(){ x.cancel(); return b; }).then(C), then it means that C is never executed, x is rejected with a cancellation error, and when the B callback returns a promise then that b value is cancelled as well.

fresheneesz commented 9 years ago

@bergus Thanks for the explanation. Sounds like its everything I'd want in cancellation!

petkaantonov commented 8 years ago

Fixed in 3.0 release

bergus commented 8 years ago

Nice! I see there still are many documentation holes to fill. I guess I'll open another issue about the questions left open in the 3.0.0 docs

Artazor commented 8 years ago

Congrats!