Clarification of execution of next() and throw() with await

[gskachkov]

During testing async iteration I faced with unexpected result:

const getPromise = ph => {
    return new Promise((resolve, reject) => {
        ph.resolve = resolve;
        ph.reject = reject;
    });
};
const promiseHolder = {};
const promise = getPromise(promiseHolder);

async function *foo() {
    yield '#1';
    var result = await promise;
    yield result + '#2';
    yield result + '#3';
};

const f = foo();

f.next().then(({ value })=> print('fulfilled #1', value));
f.next().then(({ value })=> print('fulfilled #2', value));

promiseHolder.resolve('success');

f.throw(new Error('Some error')).then(({ value })=> print('fulfilled #3', value), error=> print('reject #3', error));
f.next().then(({ value })=> print('fulfilled #4', value));

I expect following print:

fulfilled #1 #1
fulfilled #2 success#2
reject #3 Error: Some error
fulfilled #4 undefined
// But I received 
fulfilled #1 #1
reject #3 Error: Some error
fulfilled #2 success#2
fulfilled #4 undefined

As I can understand this happened because we do not directly resolve promiseCapabilty but use valueWrapperCapability:
6.4.3.3 AsyncGeneratorResolve
....
6.4.3.3.10. Perform ! PerformPromiseThen(valueWrapperCapability.[[Promise]], onFulfilled, undefined, promiseCapability)
6.4.3.3.11.Perform ! AsyncGeneratorResumeNext(generator).
and in case of throw() we print result earlier because in 6.4.3.4 AsyncGeneratorReject we reject promiseCapabilty without any wrapper.

Could you please suggest is this something wrong in my implementation or it works as expected?

Best regards,
Oleksandr

[domenic]

What implementation are you testing?

Can you edit your test code to be runnable? For example it references the undefined value ph7, and your log statements do not match up with your "I expect following print".

[gskachkov]

@domenic Oh, Sorry for code with error! Fixed.

I'm implementing this spec for JavaScriptCore

[domenic]

Thanks @gskachkov! Unfortunately I wasn't able to get to this by the end of my workday, but I'll definitely dig in on Monday. (Thoughts from @arai-a and @caitp / @GeorgNeis also welcome.)

[arai-a]

fwiw, I also get the following with WIP patch for SpiderMonkey:

fulfilled #1 #1
reject #3 Error: Some error
fulfilled #2 success#2
fulfilled #4 undefined

I'll check what's happening.

[arai-a]

here's the list of jobs queued after await (so, executing the continuation of async generator)

1. Async Iterator Value Unwrap Functions (A)
   (queued by yield result + '#2', {value: "success#2", done: false})
2. onRejected for reject #3
   (queued by AsyncGeneratorReject after yield result + '#2' throws)
3. Async Iterator Value Unwrap Functions (B)
   (queued by AsyncGeneratorResumeNext in AsyncGeneratorReject, {value: undefined, done: true})
4. onFulfilled for fulfilled # 2
   (queued by Async Iterator Value Unwrap Functions (A))
5. onFulfilled for fulfilled # 4
   (queued by Async Iterator Value Unwrap Functions (B))

so, iiuc, the out-of-order happens because of Async Iterator Value Unwrap Functions

[gskachkov]

@arai-a Thanks! So my implementation is correct :-)
@domenic So it is expected by spec that some operations would be handled in another order than they invoked, is it?

[gskachkov]

There is one more example without await and I think it also related to the 'Async Iterator Value Unwrap Functions'

async function *foo() {
    yield '#1';
};

const f = foo();

f.return('bar').then(({value})=> print('return#1', value));
f.throw(new Error('some error')).then(({value})=>print('fulfilled throw#2', value), error=>print('reject#2', error));
f.next().then(({value})=>print('fulfilled next#3', value), error=>print('reject#3', error));

I received:

reject#2 Error: some error
return#1 bar
fulfilled next#3 undefined

But without knowing internals I would expect this in following order:

return#1 bar
reject#2 Error: some error
fulfilled next#3 undefined

@arai-a Could you please double check this snipped on your WIP patch, to be sure that is not my wrong implementation?

[arai-a]

i get this

reject#2 Error: some error
return#1 bar
fulfilled next#3 undefined

[gskachkov]

@arai-a Thanks!

Sorry, I did not follow spec from start and missed discussions and possible ask already asked question, but why do we wrap fulfilled result in Unwrap function, but did not do this for reject?

[zenparsing]

I agree that the behavior is unexpected. If I understand the current spec correctly, AsyncGeneratorResolve calls AsyncGeneratorResumeNext without waiting for the yielded value to be unwrapped.

It seems to me that we should not dequeue any further items off of AsyncGeneratorQueue until the current value has been unwrapped. Originally I think we discussed the unwrapping happening "within" the yield and before AsyncGeneratorResolve is called, but perhaps there were considerations leading away from that solution.

[gskachkov]

@zenparsing Hmm, IMHO cause of the difference in the behavior, is difference in AsyncGeneratorResolve and AsyncGeneratorReject
in AsyncGeneratorResolve we have unwrap value and call resolve the promiseCapability 6.4.3.3.7 - 6.4.3.310
but for AsyncGeneratorReject we just reject the promiseCapability 6.4.3.4.5-6.4.3.4.5
So we resolve in 2 cycles(microtasks) but reject in one cycle(microtask)

For instance in my example #93 (comment)

f.return('bar').then(({value})=> print('return#1', value));
f.throw(new Error('some error')).then(({value})=>print('fulfilled throw#2', value), error=>print('reject#2', error));

When return is called, it invoked AsyncGeneratorResolve, but dequeue does not happened, because queue is empty until call throw is called. The result of return will be visible later to throw because promise callback for return will called later for one cycle/microtask later in comparison to promise callback for throw:

[zenparsing]

When return is called, it invoked AsyncGeneratorResolve, but dequeue does not happened, because queue is empty until call throw is called.

Yes, but when throw is called, the "throw" completion is dequeued immediately, before the unwrapping of the previous result value (the return) has finished.

Forcing an unnecessary unwrapping in AsyncGeneratorReject would solve the timing problem in this thread, but I'm worried that there might be a bigger problem here. What does your implementation print for this example?

async function* foo() {
  print('1');
  yield Promise.resolve('a').then(x => {
    print('2');
    return x;
  });
  print('3');
}

let iter = foo();
iter.next();
iter.next();

With value unwrapping, I would naively expect this output:

1
2
3

What would you expect? What does your implementation output?

Further: what does this do?

async function* foo() {
  yield Promise.reject(1);
  yield 2;
}

let iter = foo();
iter.next().then(print);
iter.next().then(print);

If it prints this:

Promise { <rejected> 1 }
{ value: 2, done: false }

then we have broken an important generator invariant: that a done: false cannot follow an error.

[caitp]

What would you expect? What does your implementation output?

Because I know that the Promise isn't waited for if there isn't an "await", even though it is unwrapped, I would expect 1 3 2, which is what the v8 implementation does (s/ /\n/g)

Further: what does this do? ...

First, AsyncGeneratorYield step 8: AsyncGeneratorResolve(generator, value, false).

Second, AsyncGeneratorResolve() does its stuff. At no point from this is the Generator resumed with a throw completion, even though the yield'd Promise is rejected, this is separate from the control flow of the generator. This would be different if it were yield await Promise.reject(...).then(...).

Finally, AsyncGeneratorResumeNext is invoked at the end of AsyncGeneratorResolve. Since the generator has not seen an error yet, and done===false, we resolve the second request's Promise with { value: 2, done: false }. (after actually resuming the generator with a "next" type)

EchoBeach2:v8 caitp$ cat test.js
async function* foo() {
  yield Promise.reject(1);
  yield 2;
}

let iter = foo();

function jsonprint(x) {
  %GlobalPrint(JSON.stringify(x, null, 2) + "\n");
  return x;
}

function jsonerror(x) {
  %GlobalPrint("ERROR: ");
  throw jsonprint(x);
}

iter.next().then(jsonprint, jsonerror);
iter.next().then(jsonprint, jsonerror);

EchoBeach2:v8 caitp$ out.gn/x64.release/d8 --harmony-async-iteration --allow-natives-syntax test.js
ERROR: 1
{
  "value": 2,
  "done": false
}
EchoBeach2:v8 caitp$ 

[zenparsing]

Thanks @caitp ! That's what I was afraid of.

😄

@domenic What are your thoughts on the observation that we've broken a generator invariant by allowing sequences such as:

ERROR: 1
{ "value": 2, "done": false }

[caitp]

@domenic What are your thoughts on the observation that we've broken a generator invariant by allowing sequences such as:

I think your read that the generator invariant is broken is incorrect, because the generator has not actually seen an error. The request's Promise is rejected, but as mentioned, this is entirely separate from flow control within the generator. No exception is thrown as far as the generator can see, and no rejection has been awaited.

[zenparsing]

I think your read that the generator invariant is broken is incorrect, because the generator has not actually seen an error.

For me, it's not a question of what the generator sees, but what the consumer of the generator sees when manually calling next. From the point of view of the consumer, we're now allowing sequences with multiple errors before completion, which is a different model compared to sync generators.

[caitp]

Mmm, I still don't see this as comparable to sync generators. There is no equivalent sync-generator case that sees this kind of control flow, this is true --- but so what? Promises aren't a 1:1 abstraction for sync control flow.

Now, if you wanted to make this equivalent to the sync generator case, you could simply change the generator code:

async function* foo() {
  yield await Promise.reject(1);
  yield 2;
}

But without that await, this is totally different from anything sync generators are capable of seeing at all. And, from my perspective, that's "okay". It should be possible to return some kind of error that makes sense WRT Promise API (which is forced on the consumer), but does not actually produce an error for the generator itself. And the author of the code has very simple, explicit control over this.

[zenparsing]

There is no equivalent sync-generator case that sees this kind of control flow, this is true --- but so what?

It's a fundamental change to the iteration protocol and has an impact on code which relies on that protocol. For instance, any combinator library over this proposal will have to decide what to do with interstitial errors, whereas combinators over sync iterators can always assume the "terminating error" protocol.

[caitp]

Well, you want to be able to continue iteration before a Promise has been settled, that's one of the benefits of async iteration.

Short of making all "yields" implicitly await, I don't see another way to do it, and I don't think that would be beneficial.

[zenparsing]

It's a design challenge, for sure.

Another worry I have is that, even if we're okay with the current semantics, the caller of next doesn't have a way to differentiate between a "done" rejection and a "not done" rejection.

let iter = asyncGenFunction();
iter.next().catch(err => {
  // Is the iterator at end-of-stream now or not? I can't tell.
});

The only two options I can see for solving that problem are:

• Make yield implicitly await (which means that we can't continue the iteration until the promise resolves).
• Let go of automatic value unwrapping and only unwrap as part of for-await (which means that we allow Promise<IterationResult<Promise<T>, boolean>>).

[RangerMauve]

What would a not done rejection be with regards to async generators?

[zenparsing]

@RangerMauve

According to the current spec:

async function* foo() {
  yield Promise.reject(1);
  yield 2;
}

let iter = foo();
iter.next().catch(err => {
  // This is a "not done" error, because the next call
  // to "next" will result in { value: 2, done: false }
});

[RangerMauve]

Oh weird. Unwrapping promises but not using await semantics seems kinda weird.

What happens when you yield* in sync generators on an iterator that throws an error? Does it ignore the error and continue to the next yield, or does it throw and end the iteration unless there's a catch?

If it continues to the next yield and ignores errors in the nested iterator, I'd say that not awaiting makes sense, I'm still not sure that unwrapping makes sense, though.
If it will make the generator throw, then I think that adding an implicit await to yield makes sense.

If there's not implicit await, I like the idea of being able to have an async iterator of promises over unwrapping.

[caitp]

If there's not implicit await, I like the idea of being able to have an async iterator of promises over unwrapping.

One of the problems with this is with user-defined async iterators (not generators or async-from-sync iterators), because then it's impossible to enforce.

[RangerMauve]

with user-defined async iterators ... it's impossible to enforce.

I'm not sure I understand the concern there. Is the issue that user-defined async iterators might return promises that throw errors without cleaning up the iterator? Or is it something else that I'm not getting?

If people are throwing errors from iterators and expecting iteration to continue, I'm pretty sure they're doing it wrong, and I'm not sure I see the benefit in accounting for that

[caitp]

I'm not sure I understand the concern there. Is the issue that user-defined async iterators might return promises that throw errors without cleaning up the iterator? Or is it something else that I'm not getting?

Yes. If the expectation is that rejections are final, there is no way to require that of user-defined iterators. It's not like with sync-iterators where you can just assert that they return an iterator result object from .next().

[RangerMauve]

Well with sync iterators, if there's an error, it'll just be thrown when you invoke next(), and the actual return value will be lost, right?

I think the same thing could happen here: if the async generator's next() call yields a rejected promise, then it's the same as next() throwing for sync generators. Therefore iteration should stop and the error should be thrown.

Anything that applies to user-defined async iterators not performing cleanup and error handling properly should be exactly the same for sync iterators, and if stopping iteration on throwing is good enough for sync iterators, it should be good enough for async iterators.

[caitp]

I think the same thing could happen here: if the async generator's next() call yields a rejected promise, then it's the same as next() throwing for sync generators. Therefore iteration should stop and the error should be thrown.

This isn't actually possible.

async function* foo() {
  yield Promise.resolve().then(function() {
    // At the time AsyncGeneratorResolve is performed, we have no idea that this Promise
    // will be rejected
    throw "blah";
  });
  yield 2;
}

[RangerMauve]

(reading relevant parts of the spec so I can be more informed)

[RangerMauve]

I thought I had it for a bit, and wrote up a paragraph of what I thought I understood, and I think I'm lost again. Sorry!

If I understand correctly, it's impossible because AsyncGeneratorResolve is going to make the next promise in the queue complete based on the yielded promise and will resume the generator, but I'm not sure why it's set up like that and what benefits there are over waiting for the promise to resolve or reject before resuming

[erights]

I agree we need to revisit. I am no longer convinced of my previous position.

[gskachkov]

@domenic, @erights Thanks for update! Sorry for question but I'm not fully familar with process of revisiting: Will you revisit things on next TC meeting, or it will take more time?

[zenparsing]

A related issue I see with the current spec is that it appears that for-await allows the user to see promises as the iteration value in the case where a manually created async iterator returns a promise in the "value" component.

async function f() {
  let asyncIter = {
    [Symbol.asyncIterator]() { return this; },
    next() {
      return Promise.resolve({ value: Promise.resolve(1), done: false });
    },
  };
  for await (let value of asyncIter) {
    console.log(value); // Promise<1>?
    break;
  }
}

Am I reading the spec correctly?

If so, I think it violates the user's expectation that when we see keyword "await", we assume full unwrapping, regardless of what is passed in.

In my mind, this is further evidence that the best way forward is to perform the value component unwrapping in for-await.

[gskachkov]

@zenparsing Hmm, interesting case.
IMO: I don't know why, but as developer I would not expect that internal promise would be resolved for me, just print:
// { value: Promise<1>, done: false }

[caitp]

@zenparsing good eye, I didn't notice this before. It's strange to me that the iterator result is awaited, but not the value component. On the other hand, you don't see this with async-from-sync iterators, so arguably the iterator is just implemented wrong. I dunno.

I think the expectation is more like this:

async function unwrap(value, done) {
  return { value: await value, done };
}

async function f() {
  let asyncIter = {
    [Symbol.asyncIterator]() { return this; },
    next() {
      return unwrap(Promise.resolve(1), false);
    },
  };
  for await (let value of asyncIter) {
    console.log(value); // 1
    break;
  }
}

I'm not sure how you could make this automatic or enforce it, though, other than adding an extra await (and there are probably enough of those already)

[Jamesernator]

Just a consequence I realized of value field unwrapping is that it makes the done value available later than it would've otherwise been.

Without value unwrapping this is safe to do with an async generator (and is currently how babel works so you can try it):

async function* concurrent(seq, max=8) {
    const iter = seq[Symbol.asyncIterator]()
    const promises = []
    let final
    let done = false
    for (let i=0 ; i < max ; i++) {
        // If a promise/value is received it'll still happen concurrently for an async generator
        // whereas if value unwrapping occurs this will block until the value is ready and thus won't
        // be concurrent
        const { value, done: itemDone } = await iter.next()
        if (itemDone) {
            done = true
            final = value
            break
        }
        promises.push(value)
    }

    while (!done) {
        yield await promises.shift()
        const { value, done: itemDone } = await iter.next()
        if (itemDone) {
            done = true
            final = value
        } else {
            promises.push(value)
        }
    }
    // Consume anything left over
    for (const promise of promises) {
        yield await promise
    }
    return final
}

With value unwrapping I think it's impossible to avoid over-consuming the iterator (although it's not really a big deal with this operator). e.g. the operator would look like this (and in fact this operator works regardless of whether or not value unwrapping happens at the generator level).

async function* concurrent(seq, max=8) {
    const iter = seq[Symbol.asyncIterator]()
    const promises = []
    for (let i=0 ; i < max ; i++) {
        // If value unwrapping happens then we can't inspect if we're done
        // or not until after the value is available so we might over-consume the iterator here
        promises.push(iter.next())
    }

    while (true) {
        const { value, done } = await promises.shift()
        if (done) {
            return value
        }
        yield value
        // Here we'll *always* over-consume the iterator (if its finite) because a done could already
        // be waiting in the promises
        promises.push(iter.next())
    }
}

So I'm not sure how I feel now about value unwrapping at the async generator level (still totally support value unwrapping at the for-await level though!).

[domenic]

The decision was that manually-constructed async iterators can be error prone and that we shouldn't try to patch over them in for-await (just like we don't patch over them in next() somehow). In general we don't want to treat for-await and yield* as special if at all possible.

[zenparsing]

Again, I would say that if the user sees await they should be able to assume (in a loose, intuitive way) a non-promise value.

I don't see it as a case of treating for-await as special. In fact, I found that if you just put the value unwrapping in for-await then everything falls into place. For example, you don't need the async-from-sync-generator special casing.

I'm a little unclear on how yield* plays into this. If values are only unwrapped by for-await, I don't think there's a need for value unwrapping in yield*.

[domenic]

// If value unwrapping happens then we can't inspect if we're done
// or not until after the value is available so we might over-consume the iterator here

This is intentional. You indeed can't know if you're done until the promise settles, as it might reject.

[domenic]

That is almost exactly the same example as the one given above, and the issue is the same: you can't know if the promise will reject or not.

There are easy alternate implementation strategies that will allow you to speculate that maybe the async operation will succeed, and move onward then. You can modify your code to make such speculation if you want.

If your intent is literally to yield promises for some reason, you can wrap them in an object, e.g. yield { x: delay(1000) }.

[hayes]

(I removed my comments since this is a rather large thread already, and they were not adding anything useful to the conversation)

[littledan]

To sum up, this thread lists three possible semantics:

A: AsyncGenerator.prototype.next() handles awaiting (current spec, using the Async Iterator Value Unwrap functions that opened this thread)

B: for await handles awaiting (from @zenparsing)

C yield within an async generator handles awaiting (suggested by @caitp and others)

All of these seem like plausible designs to me. Differences:

Parallelism

• In Plan A and B, if you call .next() directly (rather than using for await) you can cause more Promises to be created within the async generator, and a higher degree of parallelism
• In Plan C, any parallelism would be created explicitly by the programmer, in between yield calls.

Type of .next():

• In Plan A and C, AsyncGenerator.prototype.next() always returns a Promise of an IterationResult
• In Plan B, AsyncGenerator.prototype.next() may return a Promise of an IterationResult of a Promise

What would user intuitions be about these two properties of the semantics?

[caitp]

C yield within an async generator handles awaiting (suggested by @caitp and others)

To clarify, I don't really want this. I think the current behaviour is good enough. I can live with yielding rejected Promises not affecting generator flow control.

[domenic]

I was hoping to have time to put together a more comprehensive slide deck and summary of this issue myself in advance of the next meeting. For now I just want to say that the comment above does not cover all aspects of the issue, so that people don't get confused and re-start the discussion without full information. E.g. it doesn't deal with the confusion between the two types of rejected promises that (A) and (B) create and the way different parts of the system treat them.

[domenic]

I've put together a summary slide deck of the issue, discussing the three potential solutions I've seen under discussion so far.

This will be used at next week's TC39 meeting, but I'd be happy for people to read it and send comments ahead of time, especially about things that could be clearer or things I haven't considered.

In any case, I'm planning that we have a resolution one way or another for the larger issue by the end of the meeting. Of course then we'll be able to address the OP's smaller issue. And in the end we'll write extensive test262 tests for whatever is decided on.

[mstade]

I kept reading the slides thinking "I sure hope option 3 makes f() and g() the same." My vote counts for nothing, but anyhow I think you make a compelling argument @domenic. Nice work!

[RangerMauve]

I was initially confused by yield* in option 2 not throwing, but it made sense since it'd behave the same for manual consumers and would still throw for fot-await.

Option 3 would be the least astonishing for people to use IMO.

[RangerMauve]

With regards to option 3 allowing for async iterators that will make for-await see values that are promises, I doubt that any producers violating the contract like that will be used in the wild since it's a pretty mistake to make. A person making a custom producer might make the mistake once and catch it when doing tests and never make it again.

[rwaldron]

Resolution was option 3

[Jamesernator]

I'm fine with this resolution. It was already unreasonably difficult to write async generators which respected the concurrency of the original iterator, I never figured out how to write a map implementation that worked with my concurrent iterator using async generators (custom iterators is trivial though).

Now that unwrapping isn't happening inside async generators, Table 2 should probably be changed from saying

Additionally, the IteratorResult object that serves as a fulfillment value should have a value property whose value is not a promise (or "thenable").

to just mentioning that no special meaning is assigned to promises in the value field and that a promise in the value field may be a mistake.

[littledan]

Would this conclusion be equivalent to reverting ca6942b , or does option 3 differ from what came before this patch somehow?

[awto]

I've read the slides, but it is still not clear for me, sorry. The slides don't mention why option 0 (no unwrap in yield* and for-await) was dropped. It sounds like the least confusing version. Could anyone explain why it is dropped, please?

Why Promise.reject is treated as an exception, isn't it just a value representing a rejected promise. The same would be in async function:

async function a() {
   Promise.reject(1)
   return 1
}

async function b() {
   await Promise.reject(1)
   return 1
}

in a - it is just a value, in b - it is an exception, while, yes, if we return the value after it will be handled in next then and still will give rejected promise, but why with applies to async iterators? what if I indeed want to return a not-wrapped promise for whatever purposes?