From 6f5c14fc30b818cf08d20bdfd0a8d42776beff24 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 26 Oct 2018 12:24:16 +0200 Subject: [PATCH 1/2] stream: ended streams should resolve the async iteration Fixes: https://github.com/nodejs/node/issues/23891 --- lib/internal/streams/async_iterator.js | 5 ++++- .../test-stream-readable-async-iterators.js | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/internal/streams/async_iterator.js b/lib/internal/streams/async_iterator.js index 25b393d21f1fcb..5d7f3f0fb424b7 100644 --- a/lib/internal/streams/async_iterator.js +++ b/lib/internal/streams/async_iterator.js @@ -127,7 +127,10 @@ const createReadableStreamAsyncIterator = (stream) => { [kLastResolve]: { value: null, writable: true }, [kLastReject]: { value: null, writable: true }, [kError]: { value: null, writable: true }, - [kEnded]: { value: false, writable: true }, + [kEnded]: { + value: stream._readableState.endEmitted, + writable: true + }, [kLastPromise]: { value: null, writable: true }, // the function passed to new Promise // is cached so we avoid allocating a new diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js index ec558955c6ed18..a9431fab81654a 100644 --- a/test/parallel/test-stream-readable-async-iterators.js +++ b/test/parallel/test-stream-readable-async-iterators.js @@ -362,6 +362,24 @@ async function tests() { assert.strictEqual(e, err); } })(); + + await (async () => { + console.log('iterating on an ended stream completes'); + const r = new Readable({ + objectMode: true, + read() { + this.push('asdf'); + this.push('hehe'); + this.push(null); + } + }); + // eslint-disable-next-line no-unused-vars + for await (const a of r) { + } + // eslint-disable-next-line no-unused-vars + for await (const b of r) { + } + })(); } // to avoid missing some tests if a promise does not resolve From 3d41222b93a5aec33b785cf84b1a46bf76fd1a80 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 26 Oct 2018 12:38:08 +0200 Subject: [PATCH 2/2] stream: do not error async iterators on destroy(null) Fixes: https://github.com/nodejs/node/issues/23890 --- lib/internal/streams/async_iterator.js | 2 +- .../test-stream-readable-async-iterators.js | 23 +++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/internal/streams/async_iterator.js b/lib/internal/streams/async_iterator.js index 5d7f3f0fb424b7..90960f5a3842e3 100644 --- a/lib/internal/streams/async_iterator.js +++ b/lib/internal/streams/async_iterator.js @@ -153,7 +153,7 @@ const createReadableStreamAsyncIterator = (stream) => { }); finished(stream, (err) => { - if (err) { + if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') { const reject = iterator[kLastReject]; // reject if we are waiting for data in the Promise // returned by next() and store the error diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js index a9431fab81654a..83540de9defea3 100644 --- a/test/parallel/test-stream-readable-async-iterators.js +++ b/test/parallel/test-stream-readable-async-iterators.js @@ -335,11 +335,8 @@ async function tests() { readable.destroy(); - try { - await readable[Symbol.asyncIterator]().next(); - } catch (e) { - assert.strictEqual(e.code, 'ERR_STREAM_PREMATURE_CLOSE'); - } + const { done } = await readable[Symbol.asyncIterator]().next(); + assert.strictEqual(done, true); })(); await (async function() { @@ -380,6 +377,22 @@ async function tests() { for await (const b of r) { } })(); + + await (async () => { + console.log('destroy mid-stream does not error'); + const r = new Readable({ + objectMode: true, + read() { + this.push('asdf'); + this.push('hehe'); + } + }); + + // eslint-disable-next-line no-unused-vars + for await (const a of r) { + r.destroy(null); + } + })(); } // to avoid missing some tests if a promise does not resolve