stream: remove isPromise utility function

The function was not checking if the parameter was actually a Promise instance, but if it has a `then` method. Removing the utility function in favor of a clearer `typeof` check, handling the case when the thenable throws if then method is accessed more than once. PR-URL: #35925 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Nov 9, 2020 · b7aa5e2 · b7aa5e2
1 parent 28ed7d0
commit b7aa5e2
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 9 deletions.
diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js
@@ -5,8 +5,9 @@
 
 const {
   ArrayIsArray,
+  ReflectApply,
   SymbolAsyncIterator,
-  SymbolIterator
+  SymbolIterator,
 } = primordials;
 
 let eos;
@@ -77,10 +78,6 @@ function popCallback(streams) {
   return streams.pop();
 }
 
-function isPromise(obj) {
-  return !!(obj && typeof obj.then === 'function');
-}
-
 function isReadable(obj) {
   return !!(obj && typeof obj.pipe === 'function');
 }
@@ -222,14 +219,19 @@ function pipeline(...streams) {
         const pt = new PassThrough({
           objectMode: true
         });
-        if (isPromise(ret)) {
-          ret
-            .then((val) => {
+
+        // Handle Promises/A+ spec, `then` could be a getter that throws on
+        // second use.
+        const then = ret?.then;
+        if (typeof then === 'function') {
+          ReflectApply(then, ret, [
+            (val) => {
               value = val;
               pt.end(val);
             }, (err) => {
               pt.destroy(err);
-            });
+            }
+          ]);
         } else if (isIterable(ret, true)) {
           finishCount++;
           pump(ret, pt, finish);

diff --git a/test/parallel/test-stream-pipeline.js b/test/parallel/test-stream-pipeline.js
@@ -1240,3 +1240,24 @@ const net = require('net');
     }),
   );
 }
+{
+  function createThenable() {
+    let counter = 0;
+    return {
+      get then() {
+        if (counter++) {
+          throw new Error('Cannot access `then` more than once');
+        }
+        return Function.prototype;
+      },
+    };
+  }
+
+  pipeline(
+    function* () {
+      yield 0;
+    },
+    createThenable,
+    () => common.mustNotCall(),
+  );
+}