From ea4be72f22ce0d75307065ce95353b416bf543af Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 3 Jun 2018 11:49:41 +0200 Subject: [PATCH] child_process: swallow errors in internal communication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Much like with NODE_HANDLE_ACK, the internal protocol for communication about the sent socket should not expose its errors to the users when those async calls are not initiated by them. PR-URL: https://github.com/nodejs/node/pull/21108 Reviewed-By: Rich Trott Reviewed-By: Joyee Cheung Reviewed-By: Michaël Zasso --- lib/internal/socket_list.js | 14 +++++++------- test/parallel/parallel.status | 1 - .../test-internal-socket-list-receive.js | 6 +++--- test/parallel/test-internal-socket-list-send.js | 16 ++++++++-------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/internal/socket_list.js b/lib/internal/socket_list.js index 55077af1305121..d12686e1de107d 100644 --- a/lib/internal/socket_list.js +++ b/lib/internal/socket_list.js @@ -13,11 +13,11 @@ class SocketListSend extends EventEmitter { child.once('exit', () => this.emit('exit', this)); } - _request(msg, cmd, callback) { + _request(msg, cmd, swallowErrors, callback) { var self = this; if (!this.child.connected) return onclose(); - this.child.send(msg); + this.child._send(msg, undefined, swallowErrors); function onclose() { self.child.removeListener('internalMessage', onreply); @@ -40,14 +40,14 @@ class SocketListSend extends EventEmitter { this._request({ cmd: 'NODE_SOCKET_NOTIFY_CLOSE', key: this.key - }, 'NODE_SOCKET_ALL_CLOSED', callback); + }, 'NODE_SOCKET_ALL_CLOSED', true, callback); } getConnections(callback) { this._request({ cmd: 'NODE_SOCKET_GET_COUNT', key: this.key - }, 'NODE_SOCKET_COUNT', function(err, msg) { + }, 'NODE_SOCKET_COUNT', false, function(err, msg) { if (err) return callback(err); callback(null, msg.count); }); @@ -67,10 +67,10 @@ class SocketListReceive extends EventEmitter { function onempty(self) { if (!self.child.connected) return; - self.child.send({ + self.child._send({ cmd: 'NODE_SOCKET_ALL_CLOSED', key: self.key - }); + }, undefined, true); } this.child.on('internalMessage', (msg) => { @@ -84,7 +84,7 @@ class SocketListReceive extends EventEmitter { this.once('empty', onempty); } else if (msg.cmd === 'NODE_SOCKET_GET_COUNT') { if (!this.child.connected) return; - this.child.send({ + this.child._send({ cmd: 'NODE_SOCKET_COUNT', key: this.key, count: this.connections diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index fbb105e54cfae4..6f4c13a96f96ee 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -7,7 +7,6 @@ prefix parallel [true] # This section applies to all platforms [$system==win32] -test-child-process-fork-net-socket: PASS,FLAKY [$system==linux] diff --git a/test/parallel/test-internal-socket-list-receive.js b/test/parallel/test-internal-socket-list-receive.js index c0eb223719ae6f..e7e64887600f59 100644 --- a/test/parallel/test-internal-socket-list-receive.js +++ b/test/parallel/test-internal-socket-list-receive.js @@ -12,7 +12,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: false, - send: common.mustNotCall() + _send: common.mustNotCall() }); const list = new SocketListReceive(child, key); @@ -24,7 +24,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: common.mustCall((msg) => { + _send: common.mustCall((msg) => { assert.strictEqual(msg.cmd, 'NODE_SOCKET_ALL_CLOSED'); assert.strictEqual(msg.key, key); }) @@ -38,7 +38,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: common.mustCall((msg) => { + _send: common.mustCall((msg) => { assert.strictEqual(msg.cmd, 'NODE_SOCKET_COUNT'); assert.strictEqual(msg.key, key); assert.strictEqual(msg.count, 0); diff --git a/test/parallel/test-internal-socket-list-send.js b/test/parallel/test-internal-socket-list-send.js index 797baf43f4dd76..2e69edcedffd9f 100644 --- a/test/parallel/test-internal-socket-list-send.js +++ b/test/parallel/test-internal-socket-list-send.js @@ -16,7 +16,7 @@ const key = 'test-key'; const list = new SocketListSend(child, 'test'); - list._request('msg', 'cmd', common.mustCall((err) => { + list._request('msg', 'cmd', false, common.mustCall((err) => { common.expectsError({ code: 'ERR_CHILD_CLOSED_BEFORE_REPLY', type: Error, @@ -30,7 +30,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { + _send: function(msg) { process.nextTick(() => this.emit('internalMessage', { key, cmd: 'cmd' }) ); @@ -39,7 +39,7 @@ const key = 'test-key'; const list = new SocketListSend(child, key); - list._request('msg', 'cmd', common.mustCall((err, msg) => { + list._request('msg', 'cmd', false, common.mustCall((err, msg) => { assert.strictEqual(err, null); assert.strictEqual(msg.cmd, 'cmd'); assert.strictEqual(msg.key, key); @@ -53,12 +53,12 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { process.nextTick(() => this.emit('disconnect')); } + _send: function(msg) { process.nextTick(() => this.emit('disconnect')); } }); const list = new SocketListSend(child, key); - list._request('msg', 'cmd', common.mustCall((err) => { + list._request('msg', 'cmd', false, common.mustCall((err) => { common.expectsError({ code: 'ERR_CHILD_CLOSED_BEFORE_REPLY', type: Error, @@ -73,7 +73,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { + _send: function(msg) { assert.strictEqual(msg.cmd, 'NODE_SOCKET_NOTIFY_CLOSE'); assert.strictEqual(msg.key, key); process.nextTick(() => @@ -98,7 +98,7 @@ const key = 'test-key'; const count = 1; const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { + _send: function(msg) { assert.strictEqual(msg.cmd, 'NODE_SOCKET_GET_COUNT'); assert.strictEqual(msg.key, key); process.nextTick(() => @@ -127,7 +127,7 @@ const key = 'test-key'; const count = 1; const child = Object.assign(new EventEmitter(), { connected: true, - send: function() { + _send: function() { process.nextTick(() => { this.emit('disconnect'); this.emit('internalMessage', { key, count, cmd: 'NODE_SOCKET_COUNT' });