Skip to content

Commit

Permalink
fs: change streams to always emit close by default
Browse files Browse the repository at this point in the history
Previously due to compat reasons 'close' was only emitted if no 'error'.
This removes the compat behavior in order to properly follow expected
streams behavior.

PR-URL: #31408
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
ronag authored and Trott committed Jan 21, 2020
1 parent dcba128 commit f0d2df4
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 15 deletions.
14 changes: 1 addition & 13 deletions lib/internal/fs/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ function ReadStream(path, options) {
if (options.highWaterMark === undefined)
options.highWaterMark = 64 * 1024;

// For backwards compat do not emit close on destroy.
if (options.emitClose === undefined) {
options.emitClose = false;
}
if (options.autoDestroy === undefined) {
options.autoDestroy = false;
}
Expand Down Expand Up @@ -269,12 +265,8 @@ ReadStream.prototype._destroy = function(err, cb) {

function closeFsStream(stream, cb, err) {
stream[kFs].close(stream.fd, (er) => {
er = er || err;
cb(er);
stream.closed = true;
const s = stream._writableState || stream._readableState;
if (!er && !s.emitClose)
stream.emit('close');
cb(er || err);
});

stream.fd = null;
Expand All @@ -298,10 +290,6 @@ function WriteStream(path, options) {
// Only buffers are supported.
options.decodeStrings = true;

// For backwards compat do not emit close on destroy.
if (options.emitClose === undefined) {
options.emitClose = false;
}
if (options.autoDestroy === undefined) {
options.autoDestroy = false;
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-fs-stream-destroy-emit-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ tmpdir.refresh();

{
const stream = fs.createReadStream(__filename);
stream.on('close', common.mustNotCall());
stream.on('close', common.mustCall());
test(stream);
}

{
const stream = fs.createWriteStream(`${tmpdir.path}/dummy`);
stream.on('close', common.mustNotCall());
stream.on('close', common.mustCall());
test(stream);
}

Expand Down

0 comments on commit f0d2df4

Please sign in to comment.