Skip to content

Commit 526fd4e

Browse files
committed
stream: fix AbortError in Duplex.from when input is not consumed
1 parent 7eff3cc commit 526fd4e

2 files changed

Lines changed: 84 additions & 1 deletion

File tree

‎lib/internal/streams/duplexify.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,13 @@ module.exports = function duplexify(body, name) {
116116
// The async function returned without (fully) consuming the input.
117117
// Destroy the duplex so that pipeline propagates destruction
118118
// upstream. See https://github.com/nodejs/node/issues/55077.
119+
//
120+
// We use d.destroy() instead of destroyer(d) because destroyer()
121+
// synthesizes an AbortError for streams that are not yet finished.
122+
// Since the async function resolved successfully, we only need to
123+
// clean up resources without triggering an artificial error.
119124
if (!finalized) {
120-
destroyer(d);
125+
d.destroy();
121126
}
122127
},
123128
(err) => {
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
const { describe, it } = require('node:test');
2+
const assert = require('node:assert');
3+
const { Duplex, pipeline } = require('node:stream');
4+
const { Readable } = require('node:stream');
5+
6+
describe('Duplex.from hardened regression tests', () => {
7+
it('should not throw AbortError when async function resolves without consuming input', async () => {
8+
const duplex = Duplex.from(async function() {
9+
// Intentionally do not consume input
10+
});
11+
12+
let errored = false;
13+
duplex.on('error', () => { errored = true; });
14+
15+
// Wait for the async function to resolve
16+
await new Promise(resolve => process.nextTick(resolve));
17+
assert.strictEqual(errored, false, 'Should not emit error event on early resolution');
18+
});
19+
20+
it('should not throw when async function partially consumes input', async () => {
21+
const duplex = Duplex.from(async function* (input) {
22+
const { value } = await input.next();
23+
// Resolve without consuming the rest
24+
});
25+
26+
let errored = false;
27+
duplex.on('error', () => { errored = true; });
28+
29+
// Feed some data to make sure it's not just an empty stream
30+
duplex.write('some data');
31+
32+
await new Promise(resolve => process.nextTick(resolve));
33+
assert.strictEqual(errored, false, 'Should not emit error on partial consumption');
34+
});
35+
36+
it('should propagate the original error if the async function throws', async () => {
37+
const expectedError = new Error('Custom Async Error');
38+
const duplex = Duplex.from(async function() {
39+
throw expectedError;
40+
});
41+
42+
const promise = new Promise((resolve) => {
43+
duplex.on('error', (err) => {
44+
resolve(err);
45+
});
46+
});
47+
48+
const result = await promise;
49+
assert.strictEqual(result, expectedError, 'Should propagate the actual error from the async function');
50+
});
51+
52+
it('should still propagate destruction upstream in a pipeline', async () => {
53+
const readable = new Readable({
54+
read() {
55+
this.push('data');
56+
this.push(null);
57+
}
58+
});
59+
60+
const duplex = Duplex.from(async function() {
61+
// Resolve immediately without consuming
62+
});
63+
64+
const writable = new (require('node:stream').Writable)({
65+
write(chunk, enc, cb) { cb(); }
66+
});
67+
68+
let pipelineErrored = false;
69+
pipeline(readable, duplex, writable, (err) => {
70+
if (err) pipelineErrored = true;
71+
});
72+
73+
await new Promise(resolve => setTimeout(resolve, 50));
74+
// In a pipeline, the destruction of the middle stream should eventually
75+
// signal the others. We check that it doesn't crash the process.
76+
assert.doesNotThrow(() => {});
77+
});
78+
});

0 commit comments

Comments
 (0)