Skip to content
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

AbortablePromise #12

Open
mjackson opened this issue Aug 18, 2014 · 11 comments
Open

AbortablePromise #12

mjackson opened this issue Aug 18, 2014 · 11 comments

Comments

@mjackson
Copy link

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
Copy link
Member

domenic commented Aug 18, 2014

Seems pretty reasonable. Any reason to use

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

instead of

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

?

@mjackson
Copy link
Author

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
Copy link
Member

domenic commented Aug 18, 2014

That makes sense; very interesting.

@bergus
Copy link

bergus commented Aug 18, 2014

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
Copy link
Author

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
Copy link

bergus commented Aug 18, 2014

@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
Copy link
Author

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

@mjackson
Copy link
Author

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
Copy link

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
Copy link
Author

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

@domenic
Copy link
Member

domenic commented Oct 28, 2014

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants