Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit b7aa5e2

Browse filesBrowse files
aduh95danielleadams
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 28ed7d0 commit b7aa5e2
Copy full SHA for b7aa5e2

File tree

Expand file treeCollapse file tree

2 files changed

+32
-9
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+32
-9
lines changed
Open diff view settings
Collapse file

‎lib/internal/streams/pipeline.js‎

Copy file name to clipboardExpand all lines: lib/internal/streams/pipeline.js
+11-9Lines 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
}
@@ -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);
Collapse file

‎test/parallel/test-stream-pipeline.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-stream-pipeline.js
+21Lines changed: 21 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.