stream: fix async iterator return when destroyed earlier

[mcollina]

Unfortunately, #31314 introduced a regression that made the loop not terminate:

'use strict'

const { Readable } = require('stream')

const r = new Readable({
  read () {}
})

r.destroy()

async function run () {

  for await (let chunk of r) {
  }
}

r.on('close', () => {
  console.log('close emitted')
  run().then(() => console.log('finished'))
})

I see that as more critical as an error not being forwarded. See #31314 (comment) for more details.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[mcollina]

(I've discovered this because one of my modules broke because of this change)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/28638/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/28639/

[ronag]

LGTM.

An alternative is to add a closeEmitted property and sort out ‘finished’. But that’s a bigger change and maybe semver-major.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/28641/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/28652/

[ronag]

Note, this might only be needed for 13.x if #31509 lands on master.

[Trott]

(I've discovered this because one of my modules broke because of this change)

Perhaps we should add it to CITGM?

[mcollina]

Perhaps we should add it to CITGM?

I don't think it adds much right now. I had it using the node core stream module because readable-stream had async iterators still experimental. As for the fix, I just switched to use readable-stream instead.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/28749/

[Trott]

Landed in 64161f2...24e81d7