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 e939a87

Browse filesBrowse 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
Copy full SHA for e939a87

File tree

Expand file treeCollapse file tree

2 files changed

+57
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+57
-7
lines changed
Open diff view settings
Collapse file

‎lib/internal/streams/async_iterator.js‎

Copy file name to clipboardExpand all lines: lib/internal/streams/async_iterator.js
+16-7Lines changed: 16 additions & 7 deletions
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);
Collapse file

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

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