From 4e188b3c63f0488be03a1f723b9b50720c76fed4 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sat, 17 Aug 2019 10:51:38 +0200
Subject: [PATCH] stream: async iterator destroy compat

async iterator should not depend on internal API for better compat
with streamlike objects.

PR-URL: https://github.com/nodejs/node/pull/29176
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/internal/streams/async_iterator.js        | 25 +++++++----
 .../test-stream-readable-async-iterators.js   | 41 +++++++++++++++++++
 2 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/lib/internal/streams/async_iterator.js b/lib/internal/streams/async_iterator.js
index 0161ca98ef0..07f2191e713 100644
--- a/lib/internal/streams/async_iterator.js
+++ b/lib/internal/streams/async_iterator.js
@@ -110,17 +110,26 @@ const ReadableStreamAsyncIteratorPrototype = Object.setPrototypeOf({
   },
 
   return() {
-    // destroy(err, cb) is a private API.
-    // We can guarantee we have that here, because we control the
-    // Readable class this is attached to.
     return new Promise((resolve, reject) => {
-      this[kStream].destroy(null, (err) => {
-        if (err) {
-          reject(err);
-          return;
-        }
+      const stream = this[kStream];
+
+      // TODO(ronag): Remove this check once finished() handles
+      // already ended and/or destroyed streams.
+      const ended = stream.destroyed || stream.readableEnded ||
+        (stream._readableState && stream._readableState.endEmitted);
+      if (ended) {
         resolve(createIterResult(undefined, true));
+        return;
+      }
+
+      finished(stream, (err) => {
+        if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
+          reject(err);
+        } else {
+          resolve(createIterResult(undefined, true));
+        }
       });
+      stream.destroy();
     });
   },
 }, AsyncIteratorPrototype);
diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js
index 12971cb2363..4a63e9fd302 100644
--- a/test/parallel/test-stream-readable-async-iterators.js
+++ b/test/parallel/test-stream-readable-async-iterators.js
@@ -486,5 +486,46 @@ async function tests() {
   }
 }
 
+{
+  // AsyncIterator return should end even when destroy
+  // does not implement the callback API.
+
+  const r = new Readable({
+    objectMode: true,
+    read() {
+    }
+  });
+
+  const originalDestroy = r.destroy;
+  r.destroy = (err) => {
+    originalDestroy.call(r, err);
+  };
+  const it = r[Symbol.asyncIterator]();
+  const p = it.return();
+  r.push(null);
+  p.then(common.mustCall());
+}
+
+
+{
+  // AsyncIterator return should not error with
+  // premature close.
+
+  const r = new Readable({
+    objectMode: true,
+    read() {
+    }
+  });
+
+  const originalDestroy = r.destroy;
+  r.destroy = (err) => {
+    originalDestroy.call(r, err);
+  };
+  const it = r[Symbol.asyncIterator]();
+  const p = it.return();
+  r.emit('close');
+  p.then(common.mustCall()).catch(common.mustNotCall());
+}
+
 // To avoid missing some tests if a promise does not resolve
 tests().then(common.mustCall());