Commit 26e9c39
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
2 files changed
Lines changed: 32 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -5,8 +5,9 @@ | |||
| 5 | 5 | ||
| 6 | 6 | const { | |
| 7 | 7 | ArrayIsArray, | |
| 8 | + ReflectApply, | ||
| 8 | 9 | SymbolAsyncIterator, | |
| 9 | - SymbolIterator | ||
| 10 | + SymbolIterator, | ||
| 10 | 11 | } = primordials; | |
| 11 | 12 | ||
| 12 | 13 | let eos; | |
@@ -77,10 +78,6 @@ function popCallback(streams) { | |||
| 77 | 78 | return streams.pop(); | |
| 78 | 79 | } | |
| 79 | 80 | ||
| 80 | - function isPromise(obj) { | ||
| 81 | - return !!(obj && typeof obj.then === 'function'); | ||
| 82 | - } | ||
| 83 | - | ||
| 84 | 81 | function isReadable(obj) { | |
| 85 | 82 | return !!(obj && typeof obj.pipe === 'function'); | |
| 86 | 83 | } | |
@@ -224,14 +221,19 @@ function pipeline(...streams) { | |||
| 224 | 221 | const pt = new PassThrough({ | |
| 225 | 222 | objectMode: true | |
| 226 | 223 | }); | |
| 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) => { | ||
| 230 | 231 | value = val; | |
| 231 | 232 | pt.end(val); | |
| 232 | 233 | }, (err) => { | |
| 233 | 234 | pt.destroy(err); | |
| 234 | - }); | ||
| 235 | + } | ||
| 236 | + ]); | ||
| 235 | 237 | } else if (isIterable(ret, true)) { | |
| 236 | 238 | finishCount++; | |
| 237 | 239 | pump(ret, pt, finish); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1232,3 +1232,24 @@ const net = require('net'); | |||
| 1232 | 1232 | assert.strictEqual(res, '123'); | |
| 1233 | 1233 | })); | |
| 1234 | 1234 | } | |
| 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