Update ongoing promise in async iterator return() method

[MattiasBuelens]

@yuki3 found an interesting edge case related to async iterators.

In Firefox and Node.js (where async iteration on ReadableStream is already supported), when you run this snippet:

const readable = new ReadableStream({
  start(c) {
    c.enqueue("a");
    c.enqueue("b");
  },
  cancel() {
    console.warn("cancelled");
  }
});

const it = readable.values();
const p1 = it.next().then(x => console.log("next 1 resolved with", x));
const p2 = it.return().then(x => console.log("return resolved with", x));
const p3 = it.next().then(x => console.log("next 2 resolved with", x));

you get:

next 1 resolved with { value: 'a', done: false }
cancelled
next 2 resolved with { done: true, value: undefined }
return resolved with { done: true, value: undefined }

This is quite surprising: the second next() promise resolves before the return() promise, even though it was called after return(). Intuitively, we would expect all async iterator calls to get queued. Indeed, if you do the same thing with an async generator:

async function* test() {
  try {
    yield "a";
    yield "b";
  } finally {
    console.warn("cancelled");
  }
}

const it = test();
const p1 = it.next().then(x => console.log("next 1 resolved with", x));
const p2 = it.return().then(x => console.log("return resolved with", x));
const p3 = it.next().then(x => console.log("next 2 resolved with", x));

then the promises resolve in the order of the original calls:

next 1 resolved with { value: 'a', done: false }
cancelled
return resolved with { value: undefined, done: true }
next 2 resolved with { value: undefined, done: true }

Yuki and I believe this to be a bug in the Web IDL specification. More precisely, the return() method should update the ongoing promise, such that future calls to next() and return() are properly chained. This aligns more closely with the behavior of async generators.

Suggested commit message:

Update ongoing promise in async iterator return() method

This fixes an edge case where manually calling return(); next() could result in the next() promise resolving before the return() promise.

---

• At least two implementers are interested (and none opposed):
  • Chromium (@yuki3)
  • …
• Tests are written and can be reviewed and commented upon at:
  • Streams: test promise resolution order of async iterator web-platform-tests/wpt#44456
• Implementation bugs are filed:
  • Chromium: https://issues.chromium.org/issues/324725911
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1879771
  • WebKit: N/A
    • This spec change is only visible through ReadableStream.prototype[Symbol.asyncIterator], which WebKit doesn't support yet. (https://bugs.webkit.org/show_bug.cgi?id=194379)
  • Deno: Update ongoing promise in async iterator return() method denoland/deno#22389
  • Node.js: Update ongoing promise in async iterator return() method nodejs/node#51725
  • webidl2.js: N/A
    • No changes to the Web IDL syntax
  • widlparser: N/A
    • No changes to the Web IDL syntax
• MDN issue is filed: N/A
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[domenic]

LGTM! I think this is a bugfix and I hope someone from WebKit and/or Mozilla can quickly step up to approve. I take it that it has Chromium support already.

[annevk]

Makes sense that they should be aligned. Please file implementation bugs.