promises-aplus / cancellation-spec

Discussion and drafts of a possible promise cancellation spec.
24 stars 5 forks source link

AbortablePromise #12

Open mjackson opened 10 years ago

mjackson commented 10 years ago

I'm currently using an AbortablePromise class that I wrote for mach with good results. The API is very simple (adds only one argument to your resolver), transparently supports propagation, and doesn't make any assumptions about whether the promise is fulfilled or rejected when you abort. Also, only one new property/method is exposed on promise objects, promise.abort.

Here's an example of what it looks like to use it:

var promise = new AbortablePromise(function (resolve, reject, onAbort) {
  // Use resolve & reject as you normally would.
  var request = makeRequest( ... , function (error, response) {
    if (error) {
      reject(error);
    } else {
      resolve(response);
    }
  });

  // Use onAbort to register a promise.abort() function. It is the
  // responsibility of this function to abort the execution of the
  // promise and resolve/reject as needed.
  onAbort(function () {
    request.abort();
    reject(new Error('Request was aborted'));
  });
});

promise.abort(); // Calls the onAbort handler.

The implementation currently makes following guarantees:

  1. The abort handler may only be called once
  2. Calling either resolve or reject makes all future calls to promise.abort() a no-op
  3. The abort handler is automatically propagated to promises generated using promise.then
  4. Edit: promise.abort() propagates to children (i.e. if resolved with an abortable child promise, promise.abort() will abort the child)
  5. Callers can pass any arguments they wish through to the abort handler. This is helpful in a context-sensitive abort
  6. If the abort handler throws, the promise is rejected

It's fairly easy to layer this on top of existing promise implementations since the Promise constructor doesn't require any extra arguments, only the resolver.

domenic commented 10 years ago

Seems pretty reasonable. Any reason to use

new AbortablePromise((resolve, reject, onAbort) => {
  // ...
  onAbort(aborter);
});

instead of

new AbortablePromise((resolve, reject) => {
  // ...
}, aborter);

?

mjackson commented 10 years ago

The former gives you access to the original resolve/reject in your aborter. One key feature of this implementation is that abort doesn't automatically imply fulfilled/rejected. Instead, it's still your responsibility resolve/reject.

domenic commented 10 years ago

That makes sense; very interesting.

bergus commented 10 years ago

I wonder why you don't use Bluebird's builtin cancellation mechanism when you extend it's Promise?

Also, your lib doesn't seem to propagate cancellations to child promises:

 fulfilledPromise.then(=> new AbortablePromise(…)).abort();

Here the promise won't be aborted, although it should be.

I do however like the approach that the onAbort handler has the responsibility to call reject if it deems it to be necessary, and does that in the body of the resolver callback to the Promise constructor. In fact, I use a very similar approach in my functional Promise library :-)

mjackson commented 10 years ago

I wonder why you don't use Bluebird's builtin cancellation mechanism

The only reason I use Bluebird promises at all is because they're fast on node.js. But when I ship code to the browser, I always use the es6-promise polyfill by swapping out bluebird for es6-promise at build time. So depending on any bluebird-specific behavior (or Q, when, RSVP or whatever) is out for me.

your lib doesn't seem to propagate cancellations to child promises

Only AbortablePromises have the special then, not the other way around. If you're using a non-AbortablePromise we don't have any control over its implementation of then.

bergus commented 10 years ago

@mjackson:

your lib doesn't seem to propagate cancellations to child promises

Only AbortablePromise s have the special then, not the other way around.

However, even if you're chaining two AbortablePromises together then it won't work with your implementation :-/

If you're using a non-AbortablePromise we don't have any control over its implementation of then.

Yeah, that's the reason why we want to spec a common cancellation behaviour and API here :-)

mjackson commented 10 years ago

@bergus Ah, I see what you're saying now. Fixed :)

mjackson commented 10 years ago

Other guarantees I could make with some small tweaks:

  1. the abort handler must resolve or reject
  2. the abort handler must not resolve to another promise (seems logical since continuing after abort makes no sense)
rickharrison commented 10 years ago

I would love to have this feature! I also use es6-promise in the browser, and I use it to wrap http requests. It would be nice to be able to call abort on the promise, and then I can handle calling abort on the request library.

mjackson commented 10 years ago

@domenic What would it take to make a formal proposal here?

domenic commented 10 years ago

A formal spec write-up that is implemented in at least one or two of the major promise libraries.