From 3c38e877bbf34f017ba70c1ddda21ad4bc1ecd5a Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Mon, 14 Dec 2015 21:07:25 +0100 Subject: [PATCH] child_process: emit .send() errors in next tick This changes the behaviour of error events emitted when process.send() fails, from synchronously to asynchronously. --- lib/internal/child_process.js | 4 +- ...est-child-process-send-disconnected-err.js | 58 ++++++++++++++++++ test/parallel/test-child-process-send-err.js | 59 +++++++++++++++++++ 3 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-child-process-send-disconnected-err.js create mode 100644 test/parallel/test-child-process-send-err.js diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index ff26e8b75ab58c..57c04631446fa9 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -524,7 +524,7 @@ function setupChannel(target, channel) { if (typeof callback === 'function') { process.nextTick(callback, ex); } else { - this.emit('error', ex); // FIXME(bnoordhuis) Defer to next tick. + process.nextTick(() => this.emit('error', ex)); } return false; }; @@ -635,7 +635,7 @@ function setupChannel(target, channel) { if (typeof callback === 'function') { process.nextTick(callback, ex); } else { - this.emit('error', ex); // FIXME(bnoordhuis) Defer to next tick. + process.nextTick(() => this.emit('error', ex)); } } } diff --git a/test/parallel/test-child-process-send-disconnected-err.js b/test/parallel/test-child-process-send-disconnected-err.js new file mode 100644 index 00000000000000..8f40fa5b4fc9c2 --- /dev/null +++ b/test/parallel/test-child-process-send-disconnected-err.js @@ -0,0 +1,58 @@ +'use strict'; + +require('../common'); + +const assert = require('assert'); +const fork = require('child_process').fork; +const Pipe = process.binding('pipe_wrap').Pipe; + +const NODE_CHANNEL_FD = 3; // equals file descriptor id of IPC created + // by parent process and passed down to + // child proc with process.env.NODE_CHANNEL_FD + +if (process.argv[2] !== 'child') { + const child = fork(__filename, ['child'], { + execArgv: ['--expose-internals'] + }); + + child.on('exit', (exitCode) => process.exit(exitCode)); + + return; +} + +let errorsEmitted = 0; + +const errorSpy = (err) => { + if (err && err.message === 'channel closed') { + return errorsEmitted++; + } + + throw new Error('This callback should have been called with an error!'); +}; + +const setupChannel = require('internal/child_process').setupChannel; + +const channel = new Pipe(true); +channel.open(NODE_CHANNEL_FD); +channel.unref(); +setupChannel(process, channel); + +// disconnecting the "target" will trigger errors on .send() +process.connected = false; + +process.send('A message to send..', errorSpy); +assert.equal(errorsEmitted, 0, + 'Waits until next tick before invoking .send() callback'); + +process.nextTick(() => { + assert.equal(errorsEmitted, 1); + + process.on('error', errorSpy); + process.send('A message to send..'); + assert.equal(errorsEmitted, 1, + 'Waits until next tick before emitting error'); + + process.nextTick(() => { + assert.equal(errorsEmitted, 2); + }); +}); diff --git a/test/parallel/test-child-process-send-err.js b/test/parallel/test-child-process-send-err.js new file mode 100644 index 00000000000000..92af9cbcddc979 --- /dev/null +++ b/test/parallel/test-child-process-send-err.js @@ -0,0 +1,59 @@ +'use strict'; + +require('../common'); + +const assert = require('assert'); +const fork = require('child_process').fork; +const Pipe = process.binding('pipe_wrap').Pipe; + +const UV_UNKNOWN_ERR = -4094; +const NODE_CHANNEL_FD = 3; // equals file descriptor id of IPC created + // by parent process and passed down to + // child proc with process.env.NODE_CHANNEL_FD + +if (process.argv[2] !== 'child') { + const child = fork(__filename, ['child'], { + execArgv: ['--expose-internals'] + }); + + child.on('exit', (exitCode) => process.exit(exitCode)); + return; +} + +let errorsEmitted = 0; + +const errorSpy = (err) => { + // UV_UNKNOWN error triggered by .writeUtf8String() below + if (err && err.code === 'UNKNOWN') { + return errorsEmitted++; + } + + throw new Error('This callback should have been called with an error!'); +}; + +const setupChannel = require('internal/child_process').setupChannel; + +const channel = new Pipe(true); +channel.open(NODE_CHANNEL_FD); +channel.unref(); +setupChannel(process, channel); + +// fake UV_UNKNOWN error +channel.writeUtf8String = () => UV_UNKNOWN_ERR; + +process._send('A message to send..', undefined, false, errorSpy); +assert.equal(errorsEmitted, 0, + 'Waits until next tick before invoking ._send() callback'); + +process.nextTick(() => { + assert.equal(errorsEmitted, 1); + + process.on('error', errorSpy); + process.send('A message to send..'); + assert.equal(errorsEmitted, 1, + 'Waits until next tick before emitting error'); + + process.nextTick(() => { + assert.equal(errorsEmitted, 2); + }); +});