From 26e9c39cea741a636a32d020adf1139a7cda4012 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 2 Nov 2020 16:34:24 +0100 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/35925 Reviewed-By: Robert Nagy Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina --- lib/internal/streams/pipeline.js | 20 +++++++++++--------- test/parallel/test-stream-pipeline.js | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js index fe229bbd17ad57..f71c86f151db4a 100644 --- 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'); } @@ -224,14 +221,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 index 7cfdc4f4141571..302e99f22b4214 100644 --- a/test/parallel/test-stream-pipeline.js +++ b/test/parallel/test-stream-pipeline.js @@ -1232,3 +1232,24 @@ const net = require('net'); assert.strictEqual(res, '123'); })); } +{ + 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(), + ); +}