From 204d89d793c7b651a28f5744a39f51c032503422 Mon Sep 17 00:00:00 2001 From: Sameer Srivastava Date: Sat, 17 Mar 2018 16:24:46 +0530 Subject: [PATCH 1/5] fs,net: emitting ready for fs streams and network sockets in addition to the event names they currently use. Currently, various internal streams have different events that indicate that the underlying resource has successfully been established. This commit adds ready event for fs and net sockets to standardize on emitting ready for all of these streams. Fixes: #19304 --- lib/fs.js | 2 + lib/net.js | 1 + test/parallel/test-fs-ready-event-stream.js | 219 ++++++++++++++++++ .../test-net-socket-ready-without-cb.js | 20 ++ 4 files changed, 242 insertions(+) create mode 100644 test/parallel/test-fs-ready-event-stream.js create mode 100644 test/parallel/test-net-socket-ready-without-cb.js diff --git a/lib/fs.js b/lib/fs.js index 510369e44fd4af..69a66e1b3b2ca2 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1984,6 +1984,7 @@ ReadStream.prototype.open = function() { self.fd = fd; self.emit('open', fd); + self.emit('ready', fd); // start the flow of data. self.read(); }); @@ -2141,6 +2142,7 @@ WriteStream.prototype.open = function() { this.fd = fd; this.emit('open', fd); + this.emit('ready', fd); }); }; diff --git a/lib/net.js b/lib/net.js index 7095a5190a17ea..9247bfe3067aa1 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1181,6 +1181,7 @@ function afterConnect(status, handle, req, readable, writable) { self._unrefTimer(); self.emit('connect'); + self.emit('ready'); // start the first read, or get an immediate EOF. // this doesn't actually consume any bytes, because len=0. diff --git a/test/parallel/test-fs-ready-event-stream.js b/test/parallel/test-fs-ready-event-stream.js new file mode 100644 index 00000000000000..bced69899d3c40 --- /dev/null +++ b/test/parallel/test-fs-ready-event-stream.js @@ -0,0 +1,219 @@ +'use strict'; +const common = require('../common'); + +const assert = require('assert'); +const fs = require('fs'); +const fixtures = require('../common/fixtures'); + +const fn = fixtures.path('elipses.txt'); +const rangeFile = fixtures.path('x.txt'); + +{ + let paused = false; + let bytesRead = 0; + + const file = fs.ReadStream(fn); + const fileSize = fs.statSync(fn).size; + + assert.strictEqual(file.bytesRead, 0); + + file.on('ready', common.mustCall(function(fd) { + file.length = 0; + assert.strictEqual('number', typeof fd); + assert.strictEqual(file.bytesRead, 0); + assert.ok(file.readable); + + // GH-535 + file.pause(); + file.resume(); + file.pause(); + file.resume(); + })); + + file.on('data', function(data) { + assert.ok(data instanceof Buffer); + assert.ok(!paused); + file.length += data.length; + + bytesRead += data.length; + assert.strictEqual(file.bytesRead, bytesRead); + + paused = true; + file.pause(); + + setTimeout(function() { + paused = false; + file.resume(); + }, 10); + }); + + + file.on('end', common.mustCall(function(chunk) { + assert.strictEqual(bytesRead, fileSize); + assert.strictEqual(file.bytesRead, fileSize); + })); + + + file.on('close', common.mustCall(function() { + assert.strictEqual(bytesRead, fileSize); + assert.strictEqual(file.bytesRead, fileSize); + })); + + process.on('exit', function() { + assert.strictEqual(file.length, 30000); + }); +} + +{ + const file = fs.createReadStream(fn, { encoding: 'utf8' }); + file.length = 0; + file.on('data', function(data) { + assert.strictEqual('string', typeof data); + file.length += data.length; + + for (let i = 0; i < data.length; i++) { + // http://www.fileformat.info/info/unicode/char/2026/index.htm + assert.strictEqual('\u2026', data[i]); + } + }); + + file.on('close', common.mustCall()); + + process.on('exit', function() { + assert.strictEqual(file.length, 10000); + }); +} + +{ + const file = + fs.createReadStream(rangeFile, { bufferSize: 1, start: 1, end: 2 }); + let contentRead = ''; + file.on('data', function(data) { + contentRead += data.toString('utf-8'); + }); + file.on('end', common.mustCall(function(data) { + assert.strictEqual(contentRead, 'yz'); + })); +} + +{ + const file = fs.createReadStream(rangeFile, { bufferSize: 1, start: 1 }); + file.data = ''; + file.on('data', function(data) { + file.data += data.toString('utf-8'); + }); + file.on('end', common.mustCall(function() { + assert.strictEqual(file.data, 'yz\n'); + })); +} + +{ + // Ref: https://github.com/nodejs/node-v0.x-archive/issues/2320 + const file = fs.createReadStream(rangeFile, { bufferSize: 1.23, start: 1 }); + file.data = ''; + file.on('data', function(data) { + file.data += data.toString('utf-8'); + }); + file.on('end', common.mustCall(function() { + assert.strictEqual(file.data, 'yz\n'); + })); +} + +common.expectsError( + () => { + fs.createReadStream(rangeFile, { start: 10, end: 2 }); + }, + { + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "start" is out of range. It must be <= "end". ' + + 'Received {start: 10, end: 2}', + type: RangeError + }); + +{ + const stream = fs.createReadStream(rangeFile, { start: 0, end: 0 }); + stream.data = ''; + + stream.on('data', function(chunk) { + stream.data += chunk; + }); + + stream.on('end', common.mustCall(function() { + assert.strictEqual('x', stream.data); + })); +} + +{ + // Verify that end works when start is not specified. + const stream = new fs.createReadStream(rangeFile, { end: 1 }); + stream.data = ''; + + stream.on('data', function(chunk) { + stream.data += chunk; + }); + + stream.on('end', common.mustCall(function() { + assert.strictEqual('xy', stream.data); + })); +} + +{ + // pause and then resume immediately. + const pauseRes = fs.createReadStream(rangeFile); + pauseRes.pause(); + pauseRes.resume(); +} + +{ + let file = fs.createReadStream(rangeFile, { autoClose: false }); + let data = ''; + file.on('data', function(chunk) { data += chunk; }); + file.on('end', common.mustCall(function() { + assert.strictEqual(data, 'xyz\n'); + process.nextTick(function() { + assert(!file.closed); + assert(!file.destroyed); + fileNext(); + }); + })); + + function fileNext() { + // This will tell us if the fd is usable again or not. + file = fs.createReadStream(null, { fd: file.fd, start: 0 }); + file.data = ''; + file.on('data', function(data) { + file.data += data; + }); + file.on('end', common.mustCall(function(err) { + assert.strictEqual(file.data, 'xyz\n'); + })); + process.on('exit', function() { + assert(file.closed); + assert(file.destroyed); + }); + } +} + +{ + // Just to make sure autoClose won't close the stream because of error. + const file = fs.createReadStream(null, { fd: 13337, autoClose: false }); + file.on('data', common.mustNotCall()); + file.on('error', common.mustCall()); + process.on('exit', function() { + assert(!file.closed); + assert(!file.destroyed); + assert(file.fd); + }); +} + +{ + // Make sure stream is destroyed when file does not exist. + const file = fs.createReadStream('/path/to/file/that/does/not/exist'); + file.on('data', common.mustNotCall()); + file.on('error', common.mustCall()); + + process.on('exit', function() { + assert(!file.closed); + assert(file.destroyed); + }); +} diff --git a/test/parallel/test-net-socket-ready-without-cb.js b/test/parallel/test-net-socket-ready-without-cb.js new file mode 100644 index 00000000000000..1ad5db4760dc1e --- /dev/null +++ b/test/parallel/test-net-socket-ready-without-cb.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); + +// This test ensures that socket.connect can be called without callback +// which is optional. + +const net = require('net'); + +const server = net.createServer(common.mustCall(function(conn) { + conn.end(); + server.close(); +})).listen(0, common.mustCall(function() { + const client = new net.Socket(); + + client.on('ready', common.mustCall(function() { + client.end(); + })); + + client.connect(server.address()); +})); From 01ee0db59ddcfc9e1a28f0883ccc3d799c841ea0 Mon Sep 17 00:00:00 2001 From: Sameer Srivastava Date: Wed, 21 Mar 2018 14:51:53 +0530 Subject: [PATCH 2/5] fs,net: removed file descriptor for ready event and updated test case removed file descriptor for ready event in fs streams and updated test case to check only for ready event. Fixes: https://github.com/nodejs/node/issues/19304 --- lib/fs.js | 2 +- test/parallel/test-fs-ready-event-stream.js | 216 +------------------- 2 files changed, 3 insertions(+), 215 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 69a66e1b3b2ca2..786ddcf4133dd9 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1984,7 +1984,7 @@ ReadStream.prototype.open = function() { self.fd = fd; self.emit('open', fd); - self.emit('ready', fd); + self.emit('ready'); // start the flow of data. self.read(); }); diff --git a/test/parallel/test-fs-ready-event-stream.js b/test/parallel/test-fs-ready-event-stream.js index bced69899d3c40..fb820f6e2e7551 100644 --- a/test/parallel/test-fs-ready-event-stream.js +++ b/test/parallel/test-fs-ready-event-stream.js @@ -1,219 +1,7 @@ 'use strict'; const common = require('../common'); - -const assert = require('assert'); const fs = require('fs'); -const fixtures = require('../common/fixtures'); - -const fn = fixtures.path('elipses.txt'); -const rangeFile = fixtures.path('x.txt'); - -{ - let paused = false; - let bytesRead = 0; - - const file = fs.ReadStream(fn); - const fileSize = fs.statSync(fn).size; - - assert.strictEqual(file.bytesRead, 0); - - file.on('ready', common.mustCall(function(fd) { - file.length = 0; - assert.strictEqual('number', typeof fd); - assert.strictEqual(file.bytesRead, 0); - assert.ok(file.readable); - - // GH-535 - file.pause(); - file.resume(); - file.pause(); - file.resume(); - })); - - file.on('data', function(data) { - assert.ok(data instanceof Buffer); - assert.ok(!paused); - file.length += data.length; - - bytesRead += data.length; - assert.strictEqual(file.bytesRead, bytesRead); - - paused = true; - file.pause(); - - setTimeout(function() { - paused = false; - file.resume(); - }, 10); - }); - - - file.on('end', common.mustCall(function(chunk) { - assert.strictEqual(bytesRead, fileSize); - assert.strictEqual(file.bytesRead, fileSize); - })); - - - file.on('close', common.mustCall(function() { - assert.strictEqual(bytesRead, fileSize); - assert.strictEqual(file.bytesRead, fileSize); - })); - - process.on('exit', function() { - assert.strictEqual(file.length, 30000); - }); -} - -{ - const file = fs.createReadStream(fn, { encoding: 'utf8' }); - file.length = 0; - file.on('data', function(data) { - assert.strictEqual('string', typeof data); - file.length += data.length; - - for (let i = 0; i < data.length; i++) { - // http://www.fileformat.info/info/unicode/char/2026/index.htm - assert.strictEqual('\u2026', data[i]); - } - }); - - file.on('close', common.mustCall()); - - process.on('exit', function() { - assert.strictEqual(file.length, 10000); - }); -} - -{ - const file = - fs.createReadStream(rangeFile, { bufferSize: 1, start: 1, end: 2 }); - let contentRead = ''; - file.on('data', function(data) { - contentRead += data.toString('utf-8'); - }); - file.on('end', common.mustCall(function(data) { - assert.strictEqual(contentRead, 'yz'); - })); -} - -{ - const file = fs.createReadStream(rangeFile, { bufferSize: 1, start: 1 }); - file.data = ''; - file.on('data', function(data) { - file.data += data.toString('utf-8'); - }); - file.on('end', common.mustCall(function() { - assert.strictEqual(file.data, 'yz\n'); - })); -} - -{ - // Ref: https://github.com/nodejs/node-v0.x-archive/issues/2320 - const file = fs.createReadStream(rangeFile, { bufferSize: 1.23, start: 1 }); - file.data = ''; - file.on('data', function(data) { - file.data += data.toString('utf-8'); - }); - file.on('end', common.mustCall(function() { - assert.strictEqual(file.data, 'yz\n'); - })); -} - -common.expectsError( - () => { - fs.createReadStream(rangeFile, { start: 10, end: 2 }); - }, - { - code: 'ERR_OUT_OF_RANGE', - message: 'The value of "start" is out of range. It must be <= "end". ' + - 'Received {start: 10, end: 2}', - type: RangeError - }); - -{ - const stream = fs.createReadStream(rangeFile, { start: 0, end: 0 }); - stream.data = ''; - - stream.on('data', function(chunk) { - stream.data += chunk; - }); - - stream.on('end', common.mustCall(function() { - assert.strictEqual('x', stream.data); - })); -} - -{ - // Verify that end works when start is not specified. - const stream = new fs.createReadStream(rangeFile, { end: 1 }); - stream.data = ''; - - stream.on('data', function(chunk) { - stream.data += chunk; - }); - - stream.on('end', common.mustCall(function() { - assert.strictEqual('xy', stream.data); - })); -} - -{ - // pause and then resume immediately. - const pauseRes = fs.createReadStream(rangeFile); - pauseRes.pause(); - pauseRes.resume(); -} - -{ - let file = fs.createReadStream(rangeFile, { autoClose: false }); - let data = ''; - file.on('data', function(chunk) { data += chunk; }); - file.on('end', common.mustCall(function() { - assert.strictEqual(data, 'xyz\n'); - process.nextTick(function() { - assert(!file.closed); - assert(!file.destroyed); - fileNext(); - }); - })); - - function fileNext() { - // This will tell us if the fd is usable again or not. - file = fs.createReadStream(null, { fd: file.fd, start: 0 }); - file.data = ''; - file.on('data', function(data) { - file.data += data; - }); - file.on('end', common.mustCall(function(err) { - assert.strictEqual(file.data, 'xyz\n'); - })); - process.on('exit', function() { - assert(file.closed); - assert(file.destroyed); - }); - } -} - -{ - // Just to make sure autoClose won't close the stream because of error. - const file = fs.createReadStream(null, { fd: 13337, autoClose: false }); - file.on('data', common.mustNotCall()); - file.on('error', common.mustCall()); - process.on('exit', function() { - assert(!file.closed); - assert(!file.destroyed); - assert(file.fd); - }); -} -{ - // Make sure stream is destroyed when file does not exist. - const file = fs.createReadStream('/path/to/file/that/does/not/exist'); - file.on('data', common.mustNotCall()); - file.on('error', common.mustCall()); +const file = fs.createReadStream(__filename); - process.on('exit', function() { - assert(!file.closed); - assert(file.destroyed); - }); -} +file.on('ready', common.mustCall( () => {}, 1)); From e93c86903e630c9487a544ddc17f405a4c7cea1c Mon Sep 17 00:00:00 2001 From: Sameer Srivastava Date: Wed, 21 Mar 2018 15:00:04 +0530 Subject: [PATCH 3/5] fs,net: removed file descriptor for ready event (writestream) removed file descriptor for ready event in fs streams and updated test case to check only for ready event. Fixes: https://github.com/nodejs/node/issues/19304 --- lib/fs.js | 2 +- test/parallel/test-fs-ready-event-stream.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 786ddcf4133dd9..25fd0e2d3ef62c 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2142,7 +2142,7 @@ WriteStream.prototype.open = function() { this.fd = fd; this.emit('open', fd); - this.emit('ready', fd); + this.emit('ready'); }); }; diff --git a/test/parallel/test-fs-ready-event-stream.js b/test/parallel/test-fs-ready-event-stream.js index fb820f6e2e7551..55b864bc6dfd8f 100644 --- a/test/parallel/test-fs-ready-event-stream.js +++ b/test/parallel/test-fs-ready-event-stream.js @@ -4,4 +4,4 @@ const fs = require('fs'); const file = fs.createReadStream(__filename); -file.on('ready', common.mustCall( () => {}, 1)); +file.on('ready', common.mustCall(() => {}, 1)); From 0778b5866510d04312da97881b701e73e6d6c750 Mon Sep 17 00:00:00 2001 From: Sameer Srivastava Date: Wed, 21 Mar 2018 15:18:00 +0530 Subject: [PATCH 4/5] fs,net: updated test case to check for ready event on writestream updated test case updated test case to check for ready event on writestream Fixes: https://github.com/nodejs/node/issues/19304 --- test/parallel/test-fs-ready-event-stream.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-fs-ready-event-stream.js b/test/parallel/test-fs-ready-event-stream.js index 55b864bc6dfd8f..814760f3d11d04 100644 --- a/test/parallel/test-fs-ready-event-stream.js +++ b/test/parallel/test-fs-ready-event-stream.js @@ -1,7 +1,15 @@ + 'use strict'; const common = require('../common'); const fs = require('fs'); +const assert = require('assert'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); -const file = fs.createReadStream(__filename); +const readStream = fs.createReadStream(__filename); +readStream.on('ready', common.mustCall(() => {}, 1)); -file.on('ready', common.mustCall(() => {}, 1)); +const writeFile = path.join(tmpdir.path, 'write-fsreadyevent.txt'); +tmpdir.refresh(); +let writeStream = fs.createWriteStream(writeFile, { autoClose: true }); +writeStream.on('ready', common.mustCall(() => {}, 1)); From 008474a13a8e00f864c8e7a337873861155de64f Mon Sep 17 00:00:00 2001 From: Sameer Srivastava Date: Wed, 21 Mar 2018 15:21:45 +0530 Subject: [PATCH 5/5] fs,net: Fixed linter errors in test/parallel/test-fs-ready-event-stream.js Fixes: https://github.com/nodejs/node/issues/19304 --- test/parallel/test-fs-ready-event-stream.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-fs-ready-event-stream.js b/test/parallel/test-fs-ready-event-stream.js index 814760f3d11d04..71f7aa76e36c76 100644 --- a/test/parallel/test-fs-ready-event-stream.js +++ b/test/parallel/test-fs-ready-event-stream.js @@ -1,8 +1,6 @@ - 'use strict'; const common = require('../common'); const fs = require('fs'); -const assert = require('assert'); const path = require('path'); const tmpdir = require('../common/tmpdir'); @@ -11,5 +9,5 @@ readStream.on('ready', common.mustCall(() => {}, 1)); const writeFile = path.join(tmpdir.path, 'write-fsreadyevent.txt'); tmpdir.refresh(); -let writeStream = fs.createWriteStream(writeFile, { autoClose: true }); +const writeStream = fs.createWriteStream(writeFile, { autoClose: true }); writeStream.on('ready', common.mustCall(() => {}, 1));