Skip to content

Commit

Permalink
fix: node < 6.4.0 causing crash on 'rs' trigger
Browse files Browse the repository at this point in the history
* fix: node < 6.4.0 can't connect to stdin
* fix: child stdio when spawn is similar to fork

Fixes #1218
  • Loading branch information
remy authored Jan 9, 2018
1 parent e95ea6f commit e90f15a
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 176 deletions.
1 change: 0 additions & 1 deletion lib/monitor/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ function tryBaseDir(dir) {
function match(files, monitor, ext) {
// sort the rules by highest specificity (based on number of slashes)
// ignore rules (!) get sorted highest as they take precedent
// TODO actually check separator rules work on windows
const cwd = process.cwd();
var rules = monitor.sort(function (a, b) {
var r = b.split(path.sep).length - a.split(path.sep).length;
Expand Down
47 changes: 22 additions & 25 deletions lib/monitor/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,31 +68,30 @@ function run(options) {
var args = runCmd ? utils.stringify(executable, cmd.args) : ':';
var spawnArgs = [sh, [shFlag, args]];

if (utils.version.major === 0 && utils.version.minor < 8) {
// use the old spawn args :-\
} else {
spawnArgs.push({
env: utils.merge(options.execOptions.env, process.env),
stdio: stdio,
});
}
spawnArgs.push({
env: utils.merge(options.execOptions.env, process.env),
stdio: stdio,
});

const firstArg = cmd.args[0] || '';

// hasStdio allows us to correctly handle stdin piping
// see: https://git.io/vNtX3
const hasStdio = utils.satisfies('>= 6.4.0 || < 5');

if (
// this is a hack to avoid forking if there's a node argument being passed
// it's a short term fix, and I'm not 100% sure that `fork` is the right way
firstArg.indexOf('-') === -1 &&
firstArg !== 'inspect' &&
executable === 'node' &&
utils.version.major > 4
firstArg.indexOf('-') === -1 && // don't fork if there's a node arg
firstArg !== 'inspect' && // don't fork it's `inspect` debugger
executable === 'node' && // only fork if node
utils.version.major > 4 // only fork if node version > 4
) {
var forkArgs = cmd.args.slice(1);
var env = utils.merge(options.execOptions.env, process.env);
stdio.push('ipc');
child = fork(options.execOptions.script, forkArgs, {
env: env,
stdio: stdio,
silent: !hasStdio,
});
utils.log.detail('forking');
debug(forkArgs);
Expand Down Expand Up @@ -216,9 +215,7 @@ function run(options) {
// if the stdin piping is on, we need to unpipe, but also close stdin on
// the child, otherwise linux can throw EPIPE or ECONNRESET errors.
if (options.stdin) {
if (process.stdin.unpipe) { // node > 0.8
process.stdin.unpipe(child.stdin);
}
process.stdin.unpipe(child.stdin);
}

if (utils.isWindows) {
Expand All @@ -234,12 +231,8 @@ function run(options) {
// this seems to fix the 0.11.x issue with the "rs" restart command,
// though I'm unsure why. it seems like more data is streamed in to
// stdin after we close.
if (child && options.stdin && oldPid === child.pid) {
// this is stupid and horrible, but node 0.12 on windows blows up
// with this line, so we'll skip it entirely.
if (!utils.isWindows) {
child.stdin.end();
}
if (child && options.stdin && child.stdin && oldPid === child.pid) {
child.stdin.end();
}
callback();
});
Expand All @@ -263,8 +256,12 @@ function run(options) {

// swallow the stdin error if it happens
// ref: https://github.com/remy/nodemon/issues/1195
child.stdin.on('error', () => { });
process.stdin.pipe(child.stdin);
if (hasStdio) {
child.stdin.on('error', () => { });
process.stdin.pipe(child.stdin);
} else {
child.stdout.pipe(process.stdout);
}

bus.once('exit', function () {
if (child && process.stdin.unpipe) { // node > 0.8
Expand Down
29 changes: 11 additions & 18 deletions lib/spawn.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
var utils = require('./utils');
var merge = utils.merge;
var bus = utils.bus;
var childProcess = require('child_process');
var _spawn = childProcess.spawn;
const utils = require('./utils');
const merge = utils.merge;
const bus = utils.bus;
const spawn = require('child_process').spawn;

module.exports = function spawn(command, config, eventArgs) {
module.exports = function spawnCommand(command, config, eventArgs) {
var stdio = ['pipe', 'pipe', 'pipe'];
var child = null;

if (config.options.stdout) {
stdio = ['pipe', process.stdout, process.stderr];
Expand All @@ -20,23 +18,18 @@ module.exports = function spawn(command, config, eventArgs) {
shFlag = '/c';
}

var args = '';

if (!Array.isArray(command)) {
command = [command];
}

args = command.join(' ');
const args = command.join(' ');

if (utils.version.major >= 1 || utils.version.minor >= 8) {
var env = merge(process.env, { FILENAME: eventArgs[0] });
child = _spawn(sh, [shFlag, args], {
env: merge(config.options.execOptions.env, env),
stdio: stdio,
});
} else {
child = spawn(sh, args);
}
const env = merge(process.env, { FILENAME: eventArgs[0] });
const child = spawn(sh, [shFlag, args], {
env: merge(config.options.execOptions.env, env),
stdio: stdio,
});

if (config.required) {
var emit = {
Expand Down
3 changes: 3 additions & 0 deletions lib/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
var noop = function () { };
var path = require('path');
const semver = require('semver');
var version = process.versions.node.split('.') || [null, null, null];

var utils = (module.exports = {
semver: semver,
satisfies: test => semver.satisfies(process.versions.node, test),
version: {
major: parseInt(version[0] || 0, 10),
minor: parseInt(version[1] || 0, 10),
Expand Down
Loading

0 comments on commit e90f15a

Please sign in to comment.