From a09e59e9d8a81a76a819c7b4998b03e8ba7fb98f Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 11 Jan 2020 13:39:34 +0100 Subject: [PATCH 1/3] stream: fix async iterator destroyed error propagation There was an edge case where if _destroy calls the error callback later than one tick the iterator would complete early and not propgate the error. --- lib/internal/streams/async_iterator.js | 24 ++++++++++--------- .../test-stream-readable-async-iterators.js | 16 +++++++++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/lib/internal/streams/async_iterator.js b/lib/internal/streams/async_iterator.js index ced9c77c4f50e2..8755e22de0d00f 100644 --- a/lib/internal/streams/async_iterator.js +++ b/lib/internal/streams/async_iterator.js @@ -78,18 +78,20 @@ const ReadableStreamAsyncIteratorPrototype = ObjectSetPrototypeOf({ } if (this[kStream].destroyed) { - // We need to defer via nextTick because if .destroy(err) is - // called, the error will be emitted via nextTick, and - // we cannot guarantee that there is no error lingering around - // waiting to be emitted. return new Promise((resolve, reject) => { - process.nextTick(() => { - if (this[kError]) { - reject(this[kError]); - } else { - resolve(createIterResult(undefined, true)); - } - }); + if (this[kError]) { + reject(this[kError]); + } else if (this[kEnded]) { + resolve(createIterResult(undefined, true)); + } else { + finished(this[kStream], (err) => { + if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') { + reject(err); + } else { + resolve(createIterResult(undefined, true)); + } + }); + } }); } diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js index 4a63e9fd3022e6..2964b7318e2f0d 100644 --- a/test/parallel/test-stream-readable-async-iterators.js +++ b/test/parallel/test-stream-readable-async-iterators.js @@ -484,6 +484,22 @@ async function tests() { assert.strictEqual(e, err); })()]); } + + { + const r = new Readable({ + read () { + }, + destroy(err, callback) { + setTimeout(() => callback(new Error('asd')), 1); + } + }); + + r.destroy(); + const it = r[Symbol.asyncIterator](); + it.next().catch(common.mustCall((err) => { + assert.strictEqual(err.message, 'asd'); + })); + } } { From 5698700ef82d1d56d43dc22234c5482a7a11418c Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 11 Jan 2020 14:31:00 +0100 Subject: [PATCH 2/3] fixup: lint error --- test/parallel/test-stream-readable-async-iterators.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js index 2964b7318e2f0d..34348ebbf8b0e2 100644 --- a/test/parallel/test-stream-readable-async-iterators.js +++ b/test/parallel/test-stream-readable-async-iterators.js @@ -487,7 +487,7 @@ async function tests() { { const r = new Readable({ - read () { + read() { }, destroy(err, callback) { setTimeout(() => callback(new Error('asd')), 1); From 4e7d5a838bf68bbe3d4b6cac36a0392ad7246a00 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 12 Jan 2020 13:06:43 +0100 Subject: [PATCH 3/3] fixup: ref compare error instead of message --- test/parallel/test-stream-readable-async-iterators.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js index 34348ebbf8b0e2..ded77e01da3d9b 100644 --- a/test/parallel/test-stream-readable-async-iterators.js +++ b/test/parallel/test-stream-readable-async-iterators.js @@ -486,18 +486,19 @@ async function tests() { } { + const _err = new Error('asd'); const r = new Readable({ read() { }, destroy(err, callback) { - setTimeout(() => callback(new Error('asd')), 1); + setTimeout(() => callback(_err), 1); } }); r.destroy(); const it = r[Symbol.asyncIterator](); it.next().catch(common.mustCall((err) => { - assert.strictEqual(err.message, 'asd'); + assert.strictEqual(err, _err); })); } }