whatwg / webidl

Web IDL Standard
https://webidl.spec.whatwg.org/
Other
410 stars 164 forks source link

Update ongoing promise in async iterator return() method #1387

Closed MattiasBuelens closed 9 months ago

MattiasBuelens commented 9 months ago

@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.


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


Preview | Diff