From df1ba9734811b53713b6940e9becb7a9ea139fa6 Mon Sep 17 00:00:00 2001 From: Giorgos Ntemiris <ntemirisgiorgos3@gmail.com> Date: Mon, 19 Aug 2019 21:03:21 +0200 Subject: [PATCH 1/4] fs: allow passing true to emitClose option Allow passing true for emitClose option for fs streams. Fixes: https://github.com/nodejs/node/issues/29177 --- lib/internal/fs/streams.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index dfff08dbbd1d2a..deb79850f4924c 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -65,7 +65,9 @@ function ReadStream(path, options) { options.highWaterMark = 64 * 1024; // For backwards compat do not emit close on destroy. - options.emitClose = false; + if (options.emitClose === undefined) { + options.emitClose = false; + } Readable.call(this, options); @@ -241,7 +243,9 @@ function WriteStream(path, options) { options = copyObject(getOptions(options, {})); // For backwards compat do not emit close on destroy. - options.emitClose = false; + if (options.emitClose === undefined) { + options.emitClose = false; + } Writable.call(this, options); From 93c1f17ec2861d602b263b9450d19b127873d9fa Mon Sep 17 00:00:00 2001 From: Rich Trott <rtrott@gmail.com> Date: Wed, 21 Aug 2019 22:01:38 -0700 Subject: [PATCH 2/4] test: add `emitClose: true` tests for fs streams --- .../test-fs-stream-destroy-emit-error.js | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-fs-stream-destroy-emit-error.js b/test/parallel/test-fs-stream-destroy-emit-error.js index c0405ce5f154f0..c1db9547a8a158 100644 --- a/test/parallel/test-fs-stream-destroy-emit-error.js +++ b/test/parallel/test-fs-stream-destroy-emit-error.js @@ -6,8 +6,31 @@ const fs = require('fs'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); -test(fs.createReadStream(__filename)); -test(fs.createWriteStream(`${tmpdir.path}/dummy`)); +{ + const stream = fs.createReadStream(__filename); + stream.on('close', common.mustNotCall()); + test(stream); +} + +{ + const stream = fs.createWriteStream(`${tmpdir.path}/dummy`); + stream.on('close', common.mustNotCall()); + test(stream); +} + +{ + const stream = fs.createReadStream(__filename, { emitClose: true }); + stream.on('close', common.mustCall()); + test(stream); +} + +{ + const stream = fs.createWriteStream(`${tmpdir.path}/dummy2`, + { emitClose: true }); + stream.on('close', common.mustCall()); + test(stream); +} + function test(stream) { const err = new Error('DESTROYED'); From 3bb7bf4fd91b23684b8554ba9e0acd06446787a1 Mon Sep 17 00:00:00 2001 From: Rich Trott <rtrott@gmail.com> Date: Wed, 21 Aug 2019 22:13:56 -0700 Subject: [PATCH 3/4] doc: add emitClose option for fs streams --- doc/api/fs.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/api/fs.md b/doc/api/fs.md index d71f8afd82b878..233178cbc37f98 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1504,6 +1504,9 @@ fs.copyFileSync('source.txt', 'destination.txt', COPYFILE_EXCL); <!-- YAML added: v0.1.31 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/29212 + description: Enable `emitClose` option. - version: v11.0.0 pr-url: https://github.com/nodejs/node/pull/19898 description: Impose new restrictions on `start` and `end`, throwing @@ -1529,6 +1532,7 @@ changes: * `fd` {integer} **Default:** `null` * `mode` {integer} **Default:** `0o666` * `autoClose` {boolean} **Default:** `true` + * `emitClose` {boolean} **Default:** `false` * `start` {integer} * `end` {integer} **Default:** `Infinity` * `highWaterMark` {integer} **Default:** `64 * 1024` @@ -1555,6 +1559,10 @@ If `fd` points to a character device that only supports blocking reads available. This can prevent the process from exiting and the stream from closing naturally. +By default, the stream will not emit a `'close'` event after it has been +destroyed. (This is the opposite of the default for other `Readable` streams.) +Set the `emitClose` option to `true` to change this behavior. + ```js const fs = require('fs'); // Create a stream from some character device. @@ -1592,6 +1600,9 @@ If `options` is a string, then it specifies the encoding. <!-- YAML added: v0.1.31 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/29212 + description: Enable `emitClose` option. - version: v7.6.0 pr-url: https://github.com/nodejs/node/pull/10739 description: The `path` parameter can be a WHATWG `URL` object using @@ -1615,6 +1626,7 @@ changes: * `fd` {integer} **Default:** `null` * `mode` {integer} **Default:** `0o666` * `autoClose` {boolean} **Default:** `true` + * `emitClose` {boolean} **Default:** `false` * `start` {integer} * Returns: {fs.WriteStream} See [Writable Stream][]. @@ -1631,6 +1643,10 @@ then the file descriptor won't be closed, even if there's an error. It is the application's responsibility to close it and make sure there's no file descriptor leak. +By default, the stream will not emit a `'close'` event after it has been +destroyed. (This is the opposite of the default for other `Writable` streams.) +Set the `emitClose` option to `true` to change this behavior. + Like [`ReadStream`][], if `fd` is specified, [`WriteStream`][] will ignore the `path` argument and will use the specified file descriptor. This means that no `'open'` event will be emitted. `fd` should be blocking; non-blocking `fd`s From 08d515f01b4e1815bd3a16e4e15574cdc6a510a2 Mon Sep 17 00:00:00 2001 From: Giorgos Ntemiris <ntemirisgiorgos3@gmail.com> Date: Thu, 22 Aug 2019 20:09:34 +0200 Subject: [PATCH 4/4] doc: removed parentheses from fs doc --- doc/api/fs.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 233178cbc37f98..fb4324f968a9ec 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1560,7 +1560,7 @@ available. This can prevent the process from exiting and the stream from closing naturally. By default, the stream will not emit a `'close'` event after it has been -destroyed. (This is the opposite of the default for other `Readable` streams.) +destroyed. This is the opposite of the default for other `Readable` streams. Set the `emitClose` option to `true` to change this behavior. ```js @@ -1644,7 +1644,7 @@ It is the application's responsibility to close it and make sure there's no file descriptor leak. By default, the stream will not emit a `'close'` event after it has been -destroyed. (This is the opposite of the default for other `Writable` streams.) +destroyed. This is the opposite of the default for other `Writable` streams. Set the `emitClose` option to `true` to change this behavior. Like [`ReadStream`][], if `fd` is specified, [`WriteStream`][] will ignore the