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

Consistency with Generators and Iterators #11

Open
DanielHerr opened this issue Jul 14, 2017 · 7 comments
Open

Consistency with Generators and Iterators #11

DanielHerr opened this issue Jul 14, 2017 · 7 comments

Comments

@DanielHerr
Copy link

This proposal seems significantly more complicated than the cancellation mechanics of generator iterators, so it may be beneficial to align promise cancellation with that of generators. When async generators/iterators are finalized, they will represent an async stream of results, and it would be convenient to have a similar cancellation mechanic for promises representing a single async result.

A particular execution of a generator, represented by an iterator, can be cancelled by simply calling the iterator's return function. This is also the case for async generators/iterators. Calling the iterator return function triggers the finally clause in the generator if present. Cancellation of underlying processes can be handled in the finally clause or in the implementation of the return function for manually created iterator objects.

Promise cancellation could operate similarly. Promises could have a return or cancel function which would trigger the finally clause in a async function and callbacks added with the proposed finally promise function. The promise constructor could be extended with a third cancelled parameter for cleanup, or maybe it could simply be handled in promise.finally.

@Macil
Copy link

Macil commented Jul 21, 2017

This message linked in this project's readme describes a lot of the reasoning of the current architecture.

There is an important difference between promises and iterators: a reference to a promise does not include any capability to mutate the promise at all. Promises are read-only to code that the promises are passed to. If promises had a .cancel() or .return() method, then this wouldn't be true. Iterators were not designed with this goal: they are inherently mutable and stateful. Reading a value from an iterator consumes it and potentially changes the source's state.

I agree that I think it is important to imagine how async generators could work with cancellation tokens. I think a possibility consistent with the current cancellation and async iteration proposals could look like this, with async generator functions receiving a cancellation token as an implicit parameter:

async function* g() {
  let i = 0;
  try {
    while (true) {
      const response = await fetch(
        'https://www.example.com/echo/'+(i++),
        {cancellationToken: arguments.cancellationToken}
      );
      const text = await response.text({cancellationToken: arguments.cancellationToken});
      yield text;
    }
  } finally {
    return 'g loop ended';
  }
}

(async function() {
  const it = g();
  console.log(await it.next()); // {value: 'echo 0', done: false}
  console.log(await it.next()); // {value: 'echo 1', done: false}
  const thirdNextPromise = it.next();
  console.log(await it.return()); // {value: 'g loop ended', done: true}
  try {
    await thirdNextPromise;
  } catch (err) {
    console.log(err);
    // My first thought was that whatever error that fetch rejects with when its
    // cancellation token fires during a request gets logged here, but I'm not sure
    // if that makes sense to thread through. What if it was a different function
    // besides fetch that was awaited, and it didn't support cancellation tokens at
    // all, and it rejected with something else later? Actually, what if it successfully
    // resolved? Remember, thirdNextPromise shouldn't resolve to that, it should
    // resolve to the value next yielded by the generator, but we definitely don't
    // want the generator to continue executing.
    //
    // Instead, I think this line should get hit, and `err` should be the same error
    // that arguments.cancellationToken.throwIfCancellationRequested() would throw.
  }
})();

@rbuckton
Copy link
Collaborator

In general I think implicit parameters should be avoided as interactions can become confusing. This proposal seeks to add cancellation support without necessitating new syntax or extensions to any other existing global or built-in (like arguments).

While the intent of return() is to exit the async function as if by return, it can only do that during a yield. Attaching an implicit cancellation token to the semantics of return could easily result in confusing interactions in a function that already conflates two concepts (generators and async functions).

@Macil
Copy link

Macil commented Jul 24, 2017

Yeah, on second thought I agree it's bad that in my example that the async generator is being exited during an await instead of a yield.

@mayorovp
Copy link

mayorovp commented Jan 26, 2018

What about this example?

async function* g() {
  let i = 0;
  let prefetchedNextResponse = fetch(
      'https://www.example.com/echo/'+(i++),
      {cancellationToken: arguments.cancellationToken}
  );
  try {
    while (true) {
      const temp = prefetchedNextResponse;
      prefetchedNextResponse = fetch(
          'https://www.example.com/echo/'+(i++),
          {cancellationToken: arguments.cancellationToken}
      );
      const response = await temp;
      const text = await response.text({cancellationToken: arguments.cancellationToken});
      yield text;
    }
  } finally {
    return 'g loop ended';
  }
}

UPD: oops, edited

@Jamesernator
Copy link

Jamesernator commented Apr 13, 2018

@rbuckton What sort've mechanism would you propose for cancellation within a generator? Passing a token in as the argument to .next? Because that still effectively necessitates syntax (specifically function.sent) as there's no way to get the value to the first .next call to a generator.

e.g. Assuming function.sent one could in principle write an async iterable version of takeUntil like so:

async function* getIterator(asyncOrSyncIterable) {
    return yield* asyncIterable
}

function takeUntil(asyncIterable, promiseFactory) {
    return {
        * [Symbol.asyncIterator]() {
            // This is just a message for when the interrupt finally
            // happens if ever, we'll cancel it if the iterable completes
            const completeTokenSource = new CancelTokenSource()
            const interrupt = promiseFactory(completeTokenSource.token)
                .map(_ => {
                    return { type: 'interrupt' }
                })
            const iterator = getIterator(asyncIterable)
            const getNext = cancelToken => iterator
                .next(cancelToken)
                .map(iteratorResult => {
                    return { type: 'nextIteratorResult', iteratorResult }
                 })
            while (true) {
                 // Create our own token which we can cancel if
                 // interrupt happens
                 const nextTokenSource = new CancelTokenSource()
                 // Send our token forward to the iterator we're iterating
                 // but also race it against the token given to *us*
                 // just in case we were cancelled

                 // NOTE: The first iteration of this loop is where we need
                 // the function.sent syntax as we can't access
                 // the value sent to .next as there's no `yield` before
                 // this step
                 const outerToken = function.sent
                 const nextValue = getNext(CancelToken.race([
                     outerToken,
                     nextTokenSource.token,
                 ]))
                 const { type, ...rest } = await Promise.race([
                     interrupt,
                     nextValue,
                 ])
                 if (type === 'nextIteratorResult') {
                     const { value, done } = rest.iteratorResult
                     if (done) {
                         // Cancel the interrupt as it is no longer needed
                         completeTokenSource.cancel()
                         return
                     } else {
                         yield value
                     }
                 } else { // If interrupted
                     // Cancel the current .next() as it is no longer required
                     nextTokenSource.cancel()
                     return
                 }
            }
        }
    }
}

@rbuckton
Copy link
Collaborator

@Jamesernator it depends on how granular cancellation would need to be. If you cancel the generator only once, you would pass it as an argument to the generator. If you are intending to pass in the token for the promise to be yielded next, this would depend on either function.sent or explicitly priming the generator:

const g = takeUntil(asyncIterable, promiseFactory);
// prime the generator
g.next(); // { value: undefined, done: false }

// now start sending data
const r1 = g.next(token1);
const r2 = g.next(token2);

function takeUntil(asyncIterable, promiseFactory) {
    return {
        * [Symbol.asyncIterator]() {
            // ...snip...
            let outerToken = yield; // requires priming the generator
            while (true) {
                // ...snip...
                if (type === 'nextIteratorResult') {
                    const { value, done } = rest.iteratorResult
                    if (done) {
                        // ...snip...
                    } else {
                        outerToken = yield value
                    }
                } else { 
                    // ...snip...
                }
            }
        }
    }
}

However, this seems like less of an issue for this proposal and more of a request for an update on the function.sent proposal.

@felixfbecker
Copy link

I think in general cancellation should optimize for usage in language syntax like async/await and for await of. Cancellation tokens are very easy to pass into async function calls when awaiting the result. A .cancel() method on Promises however is way more cumbersome, because await doesn't know what to do with it, so you need to save the Promise in a variable.

Most of the examples here seem to be focused on hand-iterating async generators with .next(). I think this should focus more on for await of.

Taking this example from the async iterators proposal, would this work fine with generators?

for await (const line of readLines(filePath, { signal })) {
  console.log(line);
}
async function* readLines(path, { signal }) {
  let file = await fileOpen(path, { signal });

  try {
    while (!file.EOF) {
      yield await file.readLine({ signal });
    }
  } finally {
    await file.close({ signal });
  }
}

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

6 participants