Skip to content

Commit

Permalink
stream: remove isPromise utility function
Browse files Browse the repository at this point in the history
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 <ronagy@icloud.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
aduh95 authored and targos committed May 1, 2021
1 parent 34d8a64 commit 26e9c39
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
20 changes: 11 additions & 9 deletions lib/internal/streams/pipeline.js
Expand Up @@ -5,8 +5,9 @@

const {
ArrayIsArray,
ReflectApply,
SymbolAsyncIterator,
SymbolIterator
SymbolIterator,
} = primordials;

let eos;
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-stream-pipeline.js
Expand Up @@ -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(),
);
}

0 comments on commit 26e9c39

Please sign in to comment.