Skip to content

Commit ad98cf0

Browse files
aduh95nodejs-github-bot
authored andcommitted
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 <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent adae822 commit ad98cf0

File tree

2 files changed

+32
-9
lines changed

2 files changed

+32
-9
lines changed

lib/internal/streams/pipeline.js

+11-9
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55

66
const {
77
ArrayIsArray,
8+
ReflectApply,
89
SymbolAsyncIterator,
9-
SymbolIterator
10+
SymbolIterator,
1011
} = primordials;
1112

1213
let eos;
@@ -77,10 +78,6 @@ function popCallback(streams) {
7778
return streams.pop();
7879
}
7980

80-
function isPromise(obj) {
81-
return !!(obj && typeof obj.then === 'function');
82-
}
83-
8481
function isReadable(obj) {
8582
return !!(obj && typeof obj.pipe === 'function');
8683
}
@@ -222,14 +219,19 @@ function pipeline(...streams) {
222219
const pt = new PassThrough({
223220
objectMode: true
224221
});
225-
if (isPromise(ret)) {
226-
ret
227-
.then((val) => {
222+
223+
// Handle Promises/A+ spec, `then` could be a getter that throws on
224+
// second use.
225+
const then = ret?.then;
226+
if (typeof then === 'function') {
227+
ReflectApply(then, ret, [
228+
(val) => {
228229
value = val;
229230
pt.end(val);
230231
}, (err) => {
231232
pt.destroy(err);
232-
});
233+
}
234+
]);
233235
} else if (isIterable(ret, true)) {
234236
finishCount++;
235237
pump(ret, pt, finish);

test/parallel/test-stream-pipeline.js

+21
Original file line numberDiff line numberDiff line change
@@ -1240,3 +1240,24 @@ const net = require('net');
12401240
}),
12411241
);
12421242
}
1243+
{
1244+
function createThenable() {
1245+
let counter = 0;
1246+
return {
1247+
get then() {
1248+
if (counter++) {
1249+
throw new Error('Cannot access `then` more than once');
1250+
}
1251+
return Function.prototype;
1252+
},
1253+
};
1254+
}
1255+
1256+
pipeline(
1257+
function* () {
1258+
yield 0;
1259+
},
1260+
createThenable,
1261+
() => common.mustNotCall(),
1262+
);
1263+
}

0 commit comments

Comments
 (0)