A simpler idea?

[domenic]

Inspired by #1 (comment), what about something as simple as this? Assume underscored properties are "actually private," i.e. wouldn't be specced and are only here to illustrate. Using ES6 for brevity.

class CancellablePromise extends Promise {
  constructor(resolver, onCancelled) {
    this._onCancelled = onCancelled;
    this._isResolved = false;
    super((resolve, reject) => {
      this._reject = reject;
      const wrappedResolve = value => { resolve(value); this._isResolved = true; };
      const wrappedReject = reason => { reject(reason); this._isResolved = true; };
      resolver(wrappedResolve, wrappedReject);
    });
  }

  cancel() {
    if (typeof this._onCancelled === "function" && !this._isResolved) {
      try {
        this._onCancelled();
      } catch (e) {
        this._reject(e);
      }
    }

    this._reject(new Cancellation()); // I assume double-rejections are no-ops
  }

  then(...args) {
    // When you chain off a cancellable promise, you get another cancellable promise.
    // And cancellations propagate to progenitor promises, i.e. when calling
    // `promise.then(...).cancel()`, this ends up calling `promise.cancel()` too (see examples).
    return new this.constructor(
      (resolve, reject) => super.then(resolve, reject),
      () => this.cancel()
    );
  }

  // Allow removing cancellation privileges, for handing out to untrusted consumers. 
  uncancellable() {
    return super.then();
  }
}

// A cancellable XHR.
function get(url) {
  const xhr = new XMLHttpRequest();
  return new CancellablePromise(
    (resolve, reject) => {
      xhr.open("GET", url, true);
      xhr.onload = () => resolve(request.responseText);
      xhr.onerror = () => reject(new Error("Can't XHR"));
      xhr.send();
    },
    () => xhr.abort()
  );
}

// A cancellable JSON-parsing XHR, using cancellation propagation.
function getJSON(url) {
  return get(url).then(JSON.parse);
}

getJSON("http://example.com/data.json").cancel();
// => should work, calling the original promise's `cancel`, and thus
// the `onCancelled` passed in to the constructor, and thus `xhr.abort`.

get("http://example.com/data.json").uncancellable().cancel();
// => throws, no `cancel` method

[ForbesLindesay]

That's nice, I like the idea of just explicitly saying .uncancellable. 2 cases that need proper treatment come to mind:

Case 1:

var prom = get('https://foo.com/1')
  .then((res) => get('http://foo.com/' + res));
...
prom.cancel();

Is there any circumstance in which the second promise gets properly cancelled?

Case 2:

I think uncancellable probably isn't quite the right behaviour, I think ideally it would be:

  uncancellable() {
    return new CancellablePromise(
      resolve => resolve(this),
      () => {}
    );
  }

That is to say we should still return a promise that can be rejected with an OperationCancelled exception by calling cancel. I then think we'd need a new name for uncancellable since it wouldn't be an apt description of the behavior.

[juandopazo]

It looks good so far. I'm not thrilled about calling super in then that way because it's creating yet another intermediate promise. There could be a better way. But besides that it looks like how I'd implement it.

[skaegi]

Your "then" implicitly supports parent cancellation instead of just cancelling the work you added by calling "then".
If I'm understanding your use of super and resolve correctly I think you just want:

then(...args) {
  return new CancellablePromise(
    resolve => resolve(super(...args))
  );
}

[domenic]

@skaegi Implicitly supporting parent cancellation is the entire goal: otherwise getJSON(url).cancel() does not cancel the underlying XHR.

[domenic]

@ForbesLindesay

Case 1

That's an interesting one we should probably make possible and think more about. Good catch.

Case 2

The point of uncancellable was to revoke the ability of promise consumers to interfere with promise state, so I don't think we want to vend CancellablePromises from it.

[skaegi]

If that's what you want then write:

function getJSON(url) {
  var xhr = get(url);
  var result = xhr.then(JSON.parse);
  result.cancel = function() {
    xhr.cancel();
    result.cancel();
  }
}

Alternately....
If we want to say that parent cancellation is the default how can I just cancel the operation I added via then

[skaegi]

sigh

function getJSON(url) {
   var xhr = get(url);
   var result = xhr.then(JSON.parse);
   var resultCancel = result.cancel.bind(result);
   result.cancel = function() {
     xhr.cancel();
     resultCancel();
   }
 }

[ForbesLindesay]

@domenic I see your point re case 2

@skaegi you've just demonstrated the problem. If it's anywhere near that difficult to propagate cancellation people won't bother to do so unless they personally need cancellation in their app, which would mean cancellation was never truely available to consumers of third party libraries. It needs to propagate by default, but propagation needs to be trivial to stop.

[skaegi]

Here's a simpler and probably better variation that doesn't override cancel

function getJSON(url) {
  var xhr = get(url);
  return xhr.then(JSON.parse, function(error) {
    if (error && error.name='Cancel') {
      xhr.cancel();
    }
    throw error;
  });
}

@ForbesLindesay Our app is very much in the space that needs cancellation and is something we both use and test every day. That absolutely means that we're willing to write code like above when we're writing a "filter" however far and away the more typical case is where we've add a new operation to the chain. Whatever the propagation behavior I want to make sure it's easy to cancel just that operation without impacting the parent and the rest of it's children.

Our Deferred implementation used parent propagation for the last six months and we ultimately removed it and are much happier for it. Now when an operation is not cancelling correctly we can actually track it down instead of trying to understand why seemingly unrelated sibling calls are being rejected. With that said I'm not militantly opposed to the idea and will certainly follow whatever it is we standardize here. It would however really help if I could see a code snippet that showed how to localize the cancel propagation to just the operation then-ed to the chain.

[domenic]

@skaegi

It would however really help if I could see a code snippet that showed how to localize the cancel propagation to just the operation then-ed to the chain.

So your desired semantics are something like this?

get("http://example.com").then(function () {
  return get("http://example.org");
}).cancel(); // should only cancel the get to example.org, not example.com

?

[skaegi]

Yes that is correct.

Let me try to cover the cases as I see them...

1. For the case where the parent is pending a cancel call on a promise created by a then would call its onRejected handler with a Cancel Error. This then promise should also be removed as a listener to the parent to prevent an additional callback when the parent completes.

For your example above where the onRejected handler is undefined, the reject would propagate normally and the call to get("http://example.org") would never happen at all.

2. For the case where the parent has completed but the onFulfilled/onRejected handler for the then promise has returned a promise, the then promise assumes this state (e.g. as per the promises spec). In that case a cancel call will occur on this newly assumed promise.

For your example above this is the case where the onFulfilled handler returns the promise from get("http://example.org") and it is still pending. A cancel call in this case results in a Cancel Error getting propagated and depending on the implementation the XHR might choose to abort.

3. For the case where the promise created by a then is already completed the cancel call is a noop.

[ForbesLindesay]

If we replace then witht eh following, we track all the promises that appear inside of the then call, so that they are also cancelled when .cancel is called.

then(cb, eb, ...args) {
  var parents = [];
  function wrap(fn) {
    return function (...args) {
      var res = fn(...args);
      if (isPromise(res) && typeof res.cancel === 'function') {
        parents.push(res);
      }
      return res;
    }
  }
  return new CancellablePromise(
    resolve => resolve(super(wrap(cb), wrap(eb), ...args)),
    () => {
      for (var parent of parents) parent.cancel();
      this.cancel();
    }
  );
}

The behavior is then:

get("http://example.com")
.then(function () {
  return get("http://example.org");
})
.cancel(); // should ancel the get to example.com or example.org
           // (depending on which request is currently in progress)

To support @skaegi's request:

Add a method fork:

fork() {
  return new CancellablePromise(resolve => resolve(this), noop);
}

Which can be used:

get("http://example.com")
.fork()
.then(function () {
  return get("http://example.org");
})
.cancel(); // should only cancel the get to example.org, not example.com

[ForbesLindesay]

And equally:

get("http://example.com")
.then(function () {
  return get("http://example.org");
})
.fork()
.cancel(); // shouldn't cancel any requests, but still results in a rejected promise with the
           // cancellation exception

and

get("http://example.com")
.then(function () {
  return get("http://example.org");
})
.uncancellable()
.cancel(); // Truly doesn't cancel anything, does `cancel` even exist here?

[ForbesLindesay]

To make explanations clearer, I've separated out the promises so each one that gets created has its own name.

var A = get("http://example.com");
var B;
var C = A.then(function () {
  B = get("http://example.org");
  return B;
});

//some time later...
C.cancel();

Case 2

This seems fine:

1. A is resolved (successfully)
2. C assumes the state of B
3. C is cancelled, which cancels B because C has assumed B's state.

Case 3

This also seems fine:

1. A is resolved
2. C assumes the state of B
3. B is resolved
4. C is resolved because of 2 & 3
5. C's cancel method is called (which is a noop because C is already resolved).

Case 1

All promises are pending. C has not assumed any state.

Without propagation:

1. C is cancelled
2. Cancellation results in C being rejected
3. A resolves (successfully)
4. A calls it's then handlers
5. B is created and ultimately resolves
6. B's resolution is ignored.

With propagation:

1. C is cancelled
2. A is cancelled by propagation from C
3. B is never created
4. A and C are both rejected with a cancellation exception.

Your proposed flow

1. C is cancelled
2. Cancellation results in C being rejected
3. C magically inserts a new promise in between A and B and never resolves that promise
4. A is resolved
5. B is never created

alternatively

1. C is cancelled
2. Cancellation results in C being rejected
3. C magically replaces the callback and errback functions in the correct one of A's registered handlers with a no-op
4. A is resolved
5. B is never created

[ForbesLindesay]

With the use of the fork method we get the behavior @skaegi desires (but with an explicitly created promise to not resolve, rather than having to magic one out of thin air):

var A = get("http://example.com");
var B = A.fork();
var C;
var D = B.then(function () {
  C = get("http://example.org");
  return C;
});

//some time later...
D.cancel();

case 1

1. D is cancelled
2. B is cancelled by propagation from D
3. C is never created
4. B, C and D are all rejected with a cancellation exception.
5. A is resolved

case 2

This seems fine:

1. A is resolved (successfully)
2. B is resolved (successfully) with the state of A
3. D assumes the state of C
4. D is cancelled, which cancels C because D has assumed C's state
5. D and C are rejected with the cancellation exception

case 3

1. A is resolved (successfully)
2. B is resolved (successfully)
3. D assumes the state of C
4. C is resolved
5. D is resolved because of 3 & 4
6. D's cancel method is called (which is a noop because D is already resolved).