async iteration over a ReadableStream containing a rejected Promise will not clean up

[bakkot]

https://github.com/whatwg/streams/pull/980 defined Symbol.asyncIterator semantics for streams, including notably:

• breaking out of a for await over a readable stream will cancel/release it, and
• for awaiting over a readable stream which vends Promises will see the unwrapped values, consistent with async generators which yield Promises.

So, for example, the following code prints { item: 0 } and then "cleaning up":

(async () => {
  let stream = new ReadableStream(
    {
      start(controller) {
        controller.enqueue(Promise.resolve(0)); // NOTE: enqueuing a Promise
      },
      cancel() {
        console.log('cleaning up');
      },
    },
  );
  for await (let item of stream) {
    console.log({ item });
    break;
  }
})();

(You can run it in Firefox, which is the only implementation I'm aware of which has shipped async iteration for streams.)

But unfortunately there's an interaction between the two points above which leads to the stream dangling in the specific case that the stream vends a rejected promise:

(async () => {
  let stream = new ReadableStream(
    {
      start(controller) {
        controller.enqueue(Promise.reject(0)); // NOTE: Promise is now rejected
      },
      cancel() {
        console.log('cleaning up');
      },
    },
  );
  try {
    for await (let item of stream) {
      // not reached
    }
  } catch (e) {
    console.log('caught', e);
  }
})();

This will print caught: 0, and then nothing else. Specifically, it will not print "cleaning up" - the stream does not get cleaned up at all.

This is because the for await machinery assumes that rejected promises signal the end of iteration, as they would in an async generator, and therefore assumes that the iterable has done its own cleanup and so the for await does not need to call .return to explicitly signal cleanup (as it normally does when stopping iteration early).

---

Possibly this is sufficiently obscure that it should just be ignored. But I do think it's a bug. I believe the fix would be to do your own cleanup in this case as if the for await loop had called return - that is, add a catch handler to the promise after the "Resolve promise with chunk" step in the get the next iteration result steps algorithm, and have the catch handler invoke the asynchronous iterator return steps.

[ricea]

I was hoping that we could just require the underlying source to do cleanup in this case. But the underlying source doesn't know it is being used in async iteration. If it was being read from by a normal reader or a pipe then the rejected promise would not trigger cancellation.

So my current impression is that your solution is correct.

[MattiasBuelens]

Perhaps we should handle this in the WebIDL binding? That is, change the asynchronous iterator prototype object so the rejectSteps in step 8.7 call the asynchronous iterator return algorithm.

Otherwise, other specs that define an async iterable (such as the File System API) may run into the same issue.

[bakkot]

The normal state of affairs is that the async iterable will clean itself up when it encounters an error. For example, for streams, the "error steps" for the read request will release the reader before rejecting the returned promise - i.e., it's already doing cleanup.

The problem here arises specifically when vending a rejected promise in the non-error path, because then the readable stream does not think it has errored (and so will not do its own cleanup) but the async iteration machinery does (and so does not tell it to clean up).