Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream: fix pipeline pump #39006

Closed
wants to merge 13 commits into from
Closed
60 changes: 46 additions & 14 deletions lib/internal/streams/pipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const {
ArrayIsArray,
Promise,
SymbolAsyncIterator,
} = primordials;

Expand All @@ -13,21 +14,27 @@ let eos;
const { once } = require('internal/util');
const destroyImpl = require('internal/streams/destroy');
const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_RETURN_VALUE,
ERR_MISSING_ARGS,
ERR_STREAM_DESTROYED
} = require('internal/errors').codes;
aggregateTwoErrors,
codes: {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_RETURN_VALUE,
ERR_MISSING_ARGS,
ERR_STREAM_DESTROYED,
ERR_STREAM_PREMATURE_CLOSE,
},
} = require('internal/errors');

const { validateCallback } = require('internal/validators');

function noop() {}

const {
isIterable,
isReadable,
isStream,
} = require('internal/streams/utils');
const assert = require('internal/assert');

let EE;
let PassThrough;
let Readable;

Expand Down Expand Up @@ -102,25 +109,50 @@ async function* fromReadable(val) {
}

async function pump(iterable, writable, finish) {
if (!EE) {
EE = require('events');
}
let error;
let callback = noop;
const resume = (err) => {
error = aggregateTwoErrors(error, err);
const _callback = callback;
callback = noop;
_callback();
};
const onClose = () => {
resume(new ERR_STREAM_PREMATURE_CLOSE());
};

const waitForDrain = () => new Promise((resolve) => {
ronag marked this conversation as resolved.
Show resolved Hide resolved
assert(callback === noop);
ronag marked this conversation as resolved.
Show resolved Hide resolved
if (error || writable.destroyed) {
resolve();
} else {
callback = resolve;
}
});

writable
.on('drain', resume)
.on('error', resume)
.on('close', onClose);

try {
if (writable.writableNeedDrain === true) {
await EE.once(writable, 'drain');
if (writable.writableNeedDrain) {
await waitForDrain();
}

for await (const chunk of iterable) {
if (!writable.write(chunk)) {
if (writable.destroyed) return;
await EE.once(writable, 'drain');
await waitForDrain();
}
}
writable.end();
} catch (err) {
error = err;
error = aggregateTwoErrors(error, err);
} finally {
writable
.off('drain', resume)
.off('error', resume)
.off('close', onClose);
finish(error);
}
}
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-stream-pipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -1387,3 +1387,29 @@ const net = require('net');
assert.strictEqual(res, content);
}));
}

{
const writableLike = new EE();
writableLike.writableNeedDrain = true;

pipeline(async function *() {}, writableLike,
common.expectsError({ code: 'ERR_STREAM_PREMATURE_CLOSE' }));

writableLike.emit('close');
}

{
const writableLike = new EE();
writableLike.write = () => false;

pipeline(
async function *() {
yield null;
yield null;
},
writableLike,
common.expectsError({ code: 'ERR_STREAM_PREMATURE_CLOSE' })
);

writableLike.emit('close');
}