Skip to content

Commit

Permalink
child_process: swallow errors in internal communication
Browse files Browse the repository at this point in the history
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: #21108
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
apapirovski authored and targos committed Jun 7, 2018
1 parent 535bec8 commit 6820b49
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 19 deletions.
14 changes: 7 additions & 7 deletions lib/internal/socket_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});
Expand All @@ -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) => {
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-internal-socket-list-receive.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
})
Expand All @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions test/parallel/test-internal-socket-list-send.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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' })
);
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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(() =>
Expand All @@ -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(() =>
Expand Down Expand Up @@ -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' });
Expand Down

0 comments on commit 6820b49

Please sign in to comment.