ericelliott / speculation

JavaScript promises are made to be broken. Speculations are cancellable promises.
MIT License
197 stars 9 forks source link

Speculation as a promise wrapper #9

Open atticoos opened 7 years ago

atticoos commented 7 years ago

I love the idea behind this project, as most projects will reach a point near completion when covering remaining edges -- cancelling a pending request due to timeout or in the event of an event source triggering some change in state where the pending request is no longer needed.

The one implication I personally have in the current setup is that I have to work a bit lower-level with resolvers and rejecters. I'd like to be able to "drop this in" a promise chain without much intervention. Currently I'd need to write some higher order behavior to achieve this desired result:

function abortAfter(time) {
  return promise => {
    const cancel = new Promise(resolve => setTimeout(resolve, time);
    return speculation((resolve, reject) => {
      promise.then(resolve);
      promise.catch(reject)
    }, cancel)
  };
}

abortAfter(100)(requestPromise)
.then(...)
.catch(...)

I could see this implementation exposing an additional path as somewhat of a static initializer

speculation.fromPromise(promise, cancellable, onCancel)

Where promise is a promise generated from within the application domain, and cancellable is the promise that will resolve when the pending promise should become aborted.

Example Implementation

I'm imagining a scenario as follows:

speculation = ...

...

speculation.fromPromise = function (promise, cancel) {
  return speculation((resolve, reject) => {
    promise.then(resolve);
    promise.catch(reject); // purposefully not chaining this to avoid catching exceptions downstream
  }, cancel);
}

This would make it a lot easier to compose these pieces together with existing promises:

function makeRequest() {
  return fetch(..)
}

function abortAfter(time) {
  return promise => speculation.fromPromise(
    promise,
    new Promise(resolve => setTimeout(resolve, time))
  );
}

const withTimeout = abortAfter(30 * 1000);

withTimeout(makeRequest())
.then(..)

And otherwise simply:

speculation.fromPromise(makeRequest(), abortAfter(30 * 1000))
.then(() => ..)

The question remains: Does this belong within this project? Or would this be better composed by the dependent?

ryan-codingintrigue commented 7 years ago

I agree that we should have something built-in that helps with cancelling existing promises. Granted, they can't have a "cleanup" aspect and must run-to-completion, but if this doesn't matter it seems tedious to have to explicitly call resolve/reject.

However, since the definition of the PromiseFunction is different than that of a Promise, can't we just get speculation to accept a Promise as the first parameter so that the signature becomes:

speculation(fn: PromiseFunction | fn: Promise, shouldCancel: Promise) => Promise

And the change can simply be:

if(fn.then) {
    return speculation((resolve, reject, onCancel) => {
        fn.then(resolve);
        fn.catch(reject);
        onCancel(handleCancel);
      }, cancel);
}
atticoos commented 7 years ago

.I'm usually in the camp of having strict interfaces. This is certainly a possibility though. I suppose this will come down to a matter of preference and principle.

One implication is that we currently use arguments to process inputs, and if each argument may support different types, it might add code smell. Albeit we could pragmatically move away from arguments and move towards named arguments. Lastly, the one final thing I don't like about shared interfaces is that they don't scale well. It would become more complex over time if we want to add other initialization entry points (can't think of any off the top of my head, but still boils down to the principle against it)

We could still have onCancel accepted as a third parameter to support cleanup tasks, as seen in #10.

char0n commented 7 years ago

Hi, To join the conversation, let me first say, that it is great we finally have some "standard" to cancel the promises in the point of their creation. However I still have one use case that even this library cannot cover (or eat least I don't see how).

I have a react/redux application, and asynchronous action creators that make ajax requests. These action creators returns promises to higher levels of the applications. These higher level react components may decide that they no longer need these promises to resolve and cancel them. Currently I am using bluebird with cancel interface to be able to this + these utils that doesn't expose non standard promise features directly to the code but operate on standard/non-standard promises in polite manner.

Do you have any idea how speculation could solve my use-case ? Thanks for this library and all collaborative comments here.

ericelliott commented 7 years ago

Hi @char0n,

All my action creators return action objects, not promises. Those action objects get dispatched to the store, where it's possible that some async middlware might be listening.

That middleware handler is responsible for both the creation and potential cancellation of promises. Speculations would work well in that context, and you'll maintain the pattern of using action objects to signal user intentions and application state changes -- including the maintenance of async event state.

ericelliott commented 7 years ago

A fromPromise() utility would be a fine addition to this project. I think we should implement it as a separate utility the same way cancellable wait() works.

char0n commented 7 years ago

@ericelliott thanks for inspiration, that's an interesting concept to explore - have middleware as a promise and cancellation HUB.

smajl commented 6 years ago

Dunno how much this might be related but after some trial and error I ended up with approx this implementation of speculation:

import speculation from 'speculation';

/* Helper to simulate async request */
function fetchUser(delay: number, fail?: boolean): Promise<string> {
  return new Promise((resolve, reject) => {
    setTimeout(() => {
      fail
        ? reject('Error: Failed to fetch')
        : resolve('John Doe');
    }, delay);
  });
}

/* My shot on custom cancel token */
export class CancelToken {
    public cancel: Function;
    public cancelled: Promise<void>;
    private readonly timer?: number;

    constructor(timeout?: number) {
        this.cancelled = new Promise<void>(resolve => {
            this.cancel = resolve;
        });

        if (timeout) {
            this.timer = window.setTimeout(this.cancel, timeout);
            this.cancelled.then(() => {
                window.clearTimeout(this.timer);
            });
        }
    }
}

/* Supply your original promise and CancelToken to make the promise cancellable */
export function makePromiseCancellable<T>(fn: Promise<T>, token: CancelToken): Promise<T> {
    return speculation(
        (resolve, reject, onCancel) => {
            fn.then(resolve);
            fn.catch(reject);
            onCancel(() => {
                reject(new Error('Cancelled'));
            });
        },
        token.cancelled,
    );
}

/* Some function in your app to do something heavy... */
function getResource(time: number, maxWait: number, willFail: boolean): Promise<string> {
  // If you do some allocation here, then create CancelToken upfront
  // and use token.cancelled.then to do proper cleanup (or cleanup
  // in you promise handlers (catch, finally, whatever).

  // This returns promise, but you can easily handle things right here.
  // The token is auto-cancelled, but you can use token.cancel() to that manually.
  return makePromiseCancellable(fetchUser(time, willFail), new CancelToken(maxWait));
}

const handleSuccess = (result: string): void => console.log(`Success: ${result}`);
const handleError = (e: Error): void => console.log(`${e}`);

// success after 200ms
getResource(200, 300, false).then(handleSuccess, handleError);
// cancelled after 50ms
getResource(200, 50, false).then(handleSuccess, handleError);
// fetch error after 200ms
getResource(200, 300, true).then(handleSuccess, handleError);
// cancelled after 50ms (would fail on fetch after 200ms)
getResource(200, 50, true).then(handleSuccess, handleError);

Live here: https://stackblitz.com/edit/speculation-demo

I would like to note, that the example on the library homepage is... well... horrible. I really like the arrow functions and what not, but that thing is unreadable as hell. :) Feel free to disect anything from the example above to provide better example to new users. :)