Skip to content

Commit e939a87

Browse files
ronagBridgeAR
authored andcommitted
stream: async iterator destroy compat
async iterator should not depend on internal API for better compat with streamlike objects. PR-URL: #29176 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent eb2d96f commit e939a87

File tree

2 files changed

+57
-7
lines changed

2 files changed

+57
-7
lines changed

lib/internal/streams/async_iterator.js

+16-7
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,26 @@ const ReadableStreamAsyncIteratorPrototype = Object.setPrototypeOf({
110110
},
111111

112112
return() {
113-
// destroy(err, cb) is a private API.
114-
// We can guarantee we have that here, because we control the
115-
// Readable class this is attached to.
116113
return new Promise((resolve, reject) => {
117-
this[kStream].destroy(null, (err) => {
118-
if (err) {
114+
const stream = this[kStream];
115+
116+
// TODO(ronag): Remove this check once finished() handles
117+
// already ended and/or destroyed streams.
118+
const ended = stream.destroyed || stream.readableEnded ||
119+
(stream._readableState && stream._readableState.endEmitted);
120+
if (ended) {
121+
resolve(createIterResult(undefined, true));
122+
return;
123+
}
124+
125+
finished(stream, (err) => {
126+
if (err && err.code !== 'ERR_STREAM_PREMATURE_CLOSE') {
119127
reject(err);
120-
return;
128+
} else {
129+
resolve(createIterResult(undefined, true));
121130
}
122-
resolve(createIterResult(undefined, true));
123131
});
132+
stream.destroy();
124133
});
125134
},
126135
}, AsyncIteratorPrototype);

test/parallel/test-stream-readable-async-iterators.js

+41
Original file line numberDiff line numberDiff line change
@@ -486,5 +486,46 @@ async function tests() {
486486
}
487487
}
488488

489+
{
490+
// AsyncIterator return should end even when destroy
491+
// does not implement the callback API.
492+
493+
const r = new Readable({
494+
objectMode: true,
495+
read() {
496+
}
497+
});
498+
499+
const originalDestroy = r.destroy;
500+
r.destroy = (err) => {
501+
originalDestroy.call(r, err);
502+
};
503+
const it = r[Symbol.asyncIterator]();
504+
const p = it.return();
505+
r.push(null);
506+
p.then(common.mustCall());
507+
}
508+
509+
510+
{
511+
// AsyncIterator return should not error with
512+
// premature close.
513+
514+
const r = new Readable({
515+
objectMode: true,
516+
read() {
517+
}
518+
});
519+
520+
const originalDestroy = r.destroy;
521+
r.destroy = (err) => {
522+
originalDestroy.call(r, err);
523+
};
524+
const it = r[Symbol.asyncIterator]();
525+
const p = it.return();
526+
r.emit('close');
527+
p.then(common.mustCall()).catch(common.mustNotCall());
528+
}
529+
489530
// To avoid missing some tests if a promise does not resolve
490531
tests().then(common.mustCall());

0 commit comments

Comments
 (0)