Stream async iteration breaks when the loop body calls .destroy(null)

[TimothyGu]

• Version: master
• Platform: all
• Subsystem: stream

'use strict';

const { Readable } = require('stream');

const r = new Readable({
  objectMode: true,
  read() {
    this.push('asdf');
    this.push('hehe');
    // note: no this.push(null);
    // also reproducible with setTimeout(() => { this.push(null); }, 1000); here.
  }
});

(async () => {
  for await (const a of r) {
    r.destroy(null);
  }
  console.log('done');
})();

Prints

(node:90745) ExperimentalWarning: Readable[Symbol.asyncIterator] is an experimental feature. This feature could change at any time
(node:90745) UnhandledPromiseRejectionWarning: Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at Readable.onclose (internal/streams/end-of-stream.js:64:36)
    at Readable.emit (events.js:182:13)
    at emitCloseNT (internal/streams/destroy.js:59:8)
    at internalTickCallback (internal/process/next_tick.js:72:19)
    at process._tickCallback (internal/process/next_tick.js:47:5)
    at Function.Module.runMain (internal/modules/cjs/loader.js:763:11)
    at startup (internal/bootstrap/node.js:308:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:878:3)
(node:90745) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an asyncfunction without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:90745) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

It seems the ERR_STREAM_PREMATURE_CLOSE is erroneous, as I would expect the loop to terminate without error. Replacing r.destroy(null) with r.destroy(new Error()) would result in a rejected promise with the newly created error, as expected.

/cc @mcollina @devsnek

Refs: #23730 #23785

[TimothyGu]

Notably, the same error may occur with

(async () => {
  for await (const a of r) {
    for await (const b of r) break;
  }
  console.log('done');
})();

as the break calls the return method of the async iterator object, which calls r.destroy(null).

[targos]

What does the spec say about how the iterator should behave in the nested example?

For example, an Array iterator does not seem to be closed in this case:

[TimothyGu]

@targos Unfortunately, the spec says nothing about how return methods should function other than that they are for cleaning up resources, and the Web doesn't provide any precedent either. Array iterators don't clean up because they have nothing to clean up.

[mcollina]

This was an unintended side effect. I’ll fix it later today.

(Al that lack of prior art is why this is still experimental, sigh).