Skip to content

Commit 26e9c39

Browse files
aduh95targos
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 34d8a64 commit 26e9c39

File tree

2 files changed

+32
-9
lines changed

2 files changed

+32
-9
lines changed

lib/internal/streams/pipeline.js

Lines changed: 11 additions & 9 deletions
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
}
@@ -224,14 +221,19 @@ function pipeline(...streams) {
224221
const pt = new PassThrough({
225222
objectMode: true
226223
});
227-
if (isPromise(ret)) {
228-
ret
229-
.then((val) => {
224+
225+
// Handle Promises/A+ spec, `then` could be a getter that throws on
226+
// second use.
227+
const then = ret?.then;
228+
if (typeof then === 'function') {
229+
ReflectApply(then, ret, [
230+
(val) => {
230231
value = val;
231232
pt.end(val);
232233
}, (err) => {
233234
pt.destroy(err);
234-
});
235+
}
236+
]);
235237
} else if (isIterable(ret, true)) {
236238
finishCount++;
237239
pump(ret, pt, finish);

test/parallel/test-stream-pipeline.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,3 +1232,24 @@ const net = require('net');
12321232
assert.strictEqual(res, '123');
12331233
}));
12341234
}
1235+
{
1236+
function createThenable() {
1237+
let counter = 0;
1238+
return {
1239+
get then() {
1240+
if (counter++) {
1241+
throw new Error('Cannot access `then` more than once');
1242+
}
1243+
return Function.prototype;
1244+
},
1245+
};
1246+
}
1247+
1248+
pipeline(
1249+
function* () {
1250+
yield 0;
1251+
},
1252+
createThenable,
1253+
() => common.mustNotCall(),
1254+
);
1255+
}

0 commit comments

Comments
 (0)