From 34223ee89d890c6b361a560323b994c9c8edbb64 Mon Sep 17 00:00:00 2001 From: TZ Date: Thu, 7 Dec 2017 16:00:32 +0800 Subject: [PATCH 1/5] fix: only interrupt startup when failOnStdErr:true --- lib/cmd/start.js | 17 +++++++++++++---- test/fixtures/custom-scripts/cmd/start.js | 12 ++++++++++++ test/fixtures/custom-scripts/index.js | 15 +++++++++++++++ test/fixtures/custom-scripts/package.json | 4 ++++ test/start.test.js | 18 ++++++++++++++++-- 5 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/custom-scripts/cmd/start.js create mode 100644 test/fixtures/custom-scripts/index.js create mode 100644 test/fixtures/custom-scripts/package.json diff --git a/lib/cmd/start.js b/lib/cmd/start.js index 81b4f57..3775e65 100644 --- a/lib/cmd/start.js +++ b/lib/cmd/start.js @@ -186,16 +186,14 @@ class StartCommand extends Command { * checkStatus({ stderr, timeout }) { let count = 0; + let hasError = false; let isSuccess = true; timeout = timeout / 1000; while (!this.isReady) { try { const stat = yield fs.stat(stderr); if (stat && stat.size > 0) { - const [ stdout ] = yield exec('tail -n 100 ' + stderr); - this.logger.error(stdout); - this.logger.error('Start failed, see %s', stderr); - isSuccess = false; + hasError = true; break; } } catch (_) { @@ -212,6 +210,17 @@ class StartCommand extends Command { this.logger.log('Wait Start: %d...', ++count); } + if (hasError) { + try { + const [ stdout ] = yield exec('tail -n 100 ' + stderr); + this.logger.error(stdout); + } catch (_) { + // nothing + } + isSuccess = !this.failOnStdErr; + this.logger.error('Start got error, see %s', stderr); + } + if (!isSuccess) { this.child.kill('SIGTERM'); yield sleep(1000); diff --git a/test/fixtures/custom-scripts/cmd/start.js b/test/fixtures/custom-scripts/cmd/start.js new file mode 100644 index 0000000..8c96172 --- /dev/null +++ b/test/fixtures/custom-scripts/cmd/start.js @@ -0,0 +1,12 @@ +'use strict'; + +const Command = require('../../../../index').StartCommand; + +class StartCommand extends Command { + constructor(rawArgv) { + super(rawArgv); + this.failOnStdErr = true; + } +} + +module.exports = StartCommand; diff --git a/test/fixtures/custom-scripts/index.js b/test/fixtures/custom-scripts/index.js new file mode 100644 index 0000000..fac7b44 --- /dev/null +++ b/test/fixtures/custom-scripts/index.js @@ -0,0 +1,15 @@ +#!/usr/bin/env node + +'use strict'; + +const path = require('path'); +const Command = require('../../../index'); + +class MainCommand extends Command { + constructor(rawArgv) { + super(rawArgv); + this.load(path.join(__dirname, 'cmd')); + } +} + +new MainCommand().start(); diff --git a/test/fixtures/custom-scripts/package.json b/test/fixtures/custom-scripts/package.json new file mode 100644 index 0000000..0197c7d --- /dev/null +++ b/test/fixtures/custom-scripts/package.json @@ -0,0 +1,4 @@ +{ + "name": "custom-scripts", + "version": "1.0.0" +} \ No newline at end of file diff --git a/test/start.test.js b/test/start.test.js index 27106fe..193cb95 100644 --- a/test/start.test.js +++ b/test/start.test.js @@ -468,7 +468,7 @@ describe('test/start.test.js', () => { .end(); }); - it('should status check fail, exit with 1', function* () { + it('should status check fail, without failOnStdErr, exit with 0', function* () { mm(process.env, 'WAIT_TIME', 5000); mm(process.env, 'ERROR', 'error message'); @@ -477,7 +477,21 @@ describe('test/start.test.js', () => { yield coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1' ], { cwd }) // .debug() .expect('stderr', /nodejs.Error: error message/) - .expect('stderr', new RegExp(`Start failed, see ${stderr}`)) + .expect('stderr', new RegExp(`Start got error, see ${stderr}`)) + .expect('code', 0) + .end(); + }); + + it('should status check fail with failOnStdErr:true, exit with 1', function* () { + mm(process.env, 'WAIT_TIME', 5000); + mm(process.env, 'ERROR', 'error message'); + const customBin = path.join(__dirname, './fixtures/custom-scripts/index.js'); + const stderr = path.join(homePath, 'logs/master-stderr.log'); + + yield coffee.fork(customBin, [ 'start', '--daemon', '--workers=1' ], { cwd }) + // .debug() + .expect('stderr', /nodejs.Error: error message/) + .expect('stderr', new RegExp(`Start got error, see ${stderr}`)) .expect('code', 1) .end(); }); From 1ffcabf8e9107aed7840135e258c521146d5ad02 Mon Sep 17 00:00:00 2001 From: TZ Date: Fri, 8 Dec 2017 08:36:09 +0800 Subject: [PATCH 2/5] feat: support --ignore-error --- README.md | 1 + lib/cmd/start.js | 18 ++++++++++-------- test/fixtures/custom-scripts/cmd/start.js | 12 ------------ test/fixtures/custom-scripts/index.js | 15 --------------- test/fixtures/custom-scripts/package.json | 4 ---- test/start.test.js | 10 +++++----- 6 files changed, 16 insertions(+), 44 deletions(-) delete mode 100644 test/fixtures/custom-scripts/cmd/start.js delete mode 100644 test/fixtures/custom-scripts/index.js delete mode 100644 test/fixtures/custom-scripts/package.json diff --git a/README.md b/README.md index 1b7e96a..a9f591f 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ $ eggctl start [options] [baseDir] - `stdout` - customize stdout file, default to `$HOME/logs/master-stdout.log`. - `stderr` - customize stderr file, default to `$HOME/logs/master-stderr.log`. - `timeout` - the maximum timeout when app starts, default to 300s. + - `ignore-error` - whether ignore stderr when app starts. ### stop diff --git a/lib/cmd/start.js b/lib/cmd/start.js index 3775e65..2277cf4 100644 --- a/lib/cmd/start.js +++ b/lib/cmd/start.js @@ -59,6 +59,10 @@ class StartCommand extends Command { type: 'number', default: 300 * 1000, }, + 'ignore-error': { + description: 'whether ignore stderr when app starts', + type: 'boolean', + }, }; } @@ -121,13 +125,11 @@ class StartCommand extends Command { if (argv.env) { // if undefined, should not pass key due to `spwan`, https://github.com/nodejs/node/blob/master/lib/child_process.js#L470 env.EGG_SERVER_ENV = argv.env; - argv.env = undefined; } - // remove unused properties, alias had been remove by `removeAlias` - argv._ = undefined; - argv.$0 = undefined; - argv.daemon = undefined; + // remove unused properties from stringify, alias had been remove by `removeAlias` + const ignoreKeys = [ '_', '$0', 'env', 'daemon', 'stdout', 'stderr', 'timeout', 'ignore-error' ]; + const props = Object.keys(argv).filter(x => !ignoreKeys.includes(x)); const options = { execArgv: context.execArgv, @@ -138,7 +140,7 @@ class StartCommand extends Command { this.logger.info('Starting %s application at %s', this.frameworkName, baseDir); - const eggArgs = [ this.serverBin, JSON.stringify(argv), `--title=${argv.title}` ]; + const eggArgs = [ this.serverBin, JSON.stringify(argv, props), `--title=${argv.title}` ]; this.logger.info('Run node %s', eggArgs.join(' ')); // whether run in the background. @@ -184,7 +186,7 @@ class StartCommand extends Command { return name; } - * checkStatus({ stderr, timeout }) { + * checkStatus({ stderr, timeout, 'ignore-error': ignoreError }) { let count = 0; let hasError = false; let isSuccess = true; @@ -217,7 +219,7 @@ class StartCommand extends Command { } catch (_) { // nothing } - isSuccess = !this.failOnStdErr; + isSuccess = ignoreError; this.logger.error('Start got error, see %s', stderr); } diff --git a/test/fixtures/custom-scripts/cmd/start.js b/test/fixtures/custom-scripts/cmd/start.js deleted file mode 100644 index 8c96172..0000000 --- a/test/fixtures/custom-scripts/cmd/start.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; - -const Command = require('../../../../index').StartCommand; - -class StartCommand extends Command { - constructor(rawArgv) { - super(rawArgv); - this.failOnStdErr = true; - } -} - -module.exports = StartCommand; diff --git a/test/fixtures/custom-scripts/index.js b/test/fixtures/custom-scripts/index.js deleted file mode 100644 index fac7b44..0000000 --- a/test/fixtures/custom-scripts/index.js +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/env node - -'use strict'; - -const path = require('path'); -const Command = require('../../../index'); - -class MainCommand extends Command { - constructor(rawArgv) { - super(rawArgv); - this.load(path.join(__dirname, 'cmd')); - } -} - -new MainCommand().start(); diff --git a/test/fixtures/custom-scripts/package.json b/test/fixtures/custom-scripts/package.json deleted file mode 100644 index 0197c7d..0000000 --- a/test/fixtures/custom-scripts/package.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "custom-scripts", - "version": "1.0.0" -} \ No newline at end of file diff --git a/test/start.test.js b/test/start.test.js index 193cb95..f9d1575 100644 --- a/test/start.test.js +++ b/test/start.test.js @@ -468,13 +468,13 @@ describe('test/start.test.js', () => { .end(); }); - it('should status check fail, without failOnStdErr, exit with 0', function* () { + it('should status check fail `--ignore-error`, exit with 0', function* () { mm(process.env, 'WAIT_TIME', 5000); mm(process.env, 'ERROR', 'error message'); const stderr = path.join(homePath, 'logs/master-stderr.log'); - yield coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1' ], { cwd }) + yield coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1', '--ignore-error' ], { cwd }) // .debug() .expect('stderr', /nodejs.Error: error message/) .expect('stderr', new RegExp(`Start got error, see ${stderr}`)) @@ -482,13 +482,13 @@ describe('test/start.test.js', () => { .end(); }); - it('should status check fail with failOnStdErr:true, exit with 1', function* () { + it('should status check fail, exit with 1', function* () { mm(process.env, 'WAIT_TIME', 5000); mm(process.env, 'ERROR', 'error message'); - const customBin = path.join(__dirname, './fixtures/custom-scripts/index.js'); + const stderr = path.join(homePath, 'logs/master-stderr.log'); - yield coffee.fork(customBin, [ 'start', '--daemon', '--workers=1' ], { cwd }) + yield coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1' ], { cwd }) // .debug() .expect('stderr', /nodejs.Error: error message/) .expect('stderr', new RegExp(`Start got error, see ${stderr}`)) From 5f5d2f95f8c2e4d00ebf13b47539d8602b8e113e Mon Sep 17 00:00:00 2001 From: TZ Date: Fri, 8 Dec 2017 08:51:32 +0800 Subject: [PATCH 3/5] style: use custom stringify --- lib/cmd/start.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/cmd/start.js b/lib/cmd/start.js index 2277cf4..dc33e63 100644 --- a/lib/cmd/start.js +++ b/lib/cmd/start.js @@ -127,10 +127,6 @@ class StartCommand extends Command { env.EGG_SERVER_ENV = argv.env; } - // remove unused properties from stringify, alias had been remove by `removeAlias` - const ignoreKeys = [ '_', '$0', 'env', 'daemon', 'stdout', 'stderr', 'timeout', 'ignore-error' ]; - const props = Object.keys(argv).filter(x => !ignoreKeys.includes(x)); - const options = { execArgv: context.execArgv, env, @@ -140,7 +136,9 @@ class StartCommand extends Command { this.logger.info('Starting %s application at %s', this.frameworkName, baseDir); - const eggArgs = [ this.serverBin, JSON.stringify(argv, props), `--title=${argv.title}` ]; + // remove unused properties from stringify, alias had been remove by `removeAlias` + const ignoreKeys = [ '_', '$0', 'env', 'daemon', 'stdout', 'stderr', 'timeout', 'ignore-error' ]; + const eggArgs = [ this.serverBin, stringify(argv, ignoreKeys), `--title=${argv.title}` ]; this.logger.info('Run node %s', eggArgs.join(' ')); // whether run in the background. @@ -244,4 +242,14 @@ function* getRotatelog(logfile) { return yield fs.open(logfile, 'a'); } +function stringify(obj, ignore) { + const result = {}; + Object.keys(obj).forEach(key => { + if (!ignore.includes(key)) { + result[key] = obj[key]; + } + }); + return JSON.stringify(result); +} + module.exports = StartCommand; From 621a1497d819ceaec0daec00894c4617e4127269 Mon Sep 17 00:00:00 2001 From: TZ Date: Fri, 8 Dec 2017 10:06:04 +0800 Subject: [PATCH 4/5] chore: code style --- lib/cmd/start.js | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/lib/cmd/start.js b/lib/cmd/start.js index dc33e63..621b9b1 100644 --- a/lib/cmd/start.js +++ b/lib/cmd/start.js @@ -71,15 +71,16 @@ class StartCommand extends Command { } * run(context) { - const argv = Object.assign({}, context.argv); + const { argv, env, cwd, execArgv } = context; + const HOME = homedir(); const logDir = path.join(HOME, 'logs'); // egg-script start // egg-script start ./server // egg-script start /opt/app - let baseDir = argv._[0] || context.cwd; - if (!path.isAbsolute(baseDir)) baseDir = path.join(context.cwd, baseDir); + let baseDir = argv._[0] || cwd; + if (!path.isAbsolute(baseDir)) baseDir = path.join(cwd, baseDir); argv.baseDir = baseDir; const isDaemon = argv.daemon; @@ -97,24 +98,18 @@ class StartCommand extends Command { argv.stdout = argv.stdout || path.join(logDir, 'master-stdout.log'); argv.stderr = argv.stderr || path.join(logDir, 'master-stderr.log'); - const env = context.env; + // normalize env env.HOME = HOME; env.NODE_ENV = 'production'; - // adjust env for win - const currentPATH = env.PATH || env.Path; - // for nodeinstall - let newPATH = `${path.join(baseDir, 'node_modules/.bin')}${path.delimiter}`; - // support `${baseDir}/.node/bin` - const customNodeBinDir = path.join(baseDir, '.node/bin'); - if (yield fs.exists(customNodeBinDir)) { - newPATH += `${customNodeBinDir}${path.delimiter}`; - } - if (currentPATH) { - env.PATH = `${newPATH}${currentPATH}`; - } else { - env.PATH = newPATH; - } + env.PATH = [ + // for nodeinstall + path.join(baseDir, 'node_modules/.bin'), + // support `.node/bin`, due to npm5 will remove `node_modules/.bin` + path.join(baseDir, '.node/bin'), + // adjust env for win + env.PATH || env.Path, + ].filter(x => !!x).join(path.delimiter); // for alinode env.ENABLE_NODE_LOG = 'YES'; @@ -128,7 +123,7 @@ class StartCommand extends Command { } const options = { - execArgv: context.execArgv, + execArgv, env, stdio: 'inherit', detached: false, @@ -151,6 +146,7 @@ class StartCommand extends Command { const child = this.child = spawn('node', eggArgs, options); this.isReady = false; child.on('message', msg => { + /* istanbul ignore else */ if (msg && msg.action === 'egg-ready') { this.isReady = true; this.logger.info('%s started on %s', this.frameworkName, msg.data.address); @@ -177,6 +173,7 @@ class StartCommand extends Command { let name = 'egg'; try { const pkg = require(pkgPath); + /* istanbul ignore else */ if (pkg.name) name = pkg.name; } catch (_) { /* istanbul next */ From 26a80588792cc93d81fe84a893029fd28120a4cb Mon Sep 17 00:00:00 2001 From: TZ Date: Mon, 11 Dec 2017 11:46:10 +0800 Subject: [PATCH 5/5] refactor: ignore-stderr --- README.md | 2 +- lib/cmd/start.js | 8 ++++---- test/start.test.js | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index a9f591f..2631494 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ $ eggctl start [options] [baseDir] - `stdout` - customize stdout file, default to `$HOME/logs/master-stdout.log`. - `stderr` - customize stderr file, default to `$HOME/logs/master-stderr.log`. - `timeout` - the maximum timeout when app starts, default to 300s. - - `ignore-error` - whether ignore stderr when app starts. + - `ignore-stderr` - whether ignore stderr when app starts. ### stop diff --git a/lib/cmd/start.js b/lib/cmd/start.js index 621b9b1..720a2cb 100644 --- a/lib/cmd/start.js +++ b/lib/cmd/start.js @@ -59,7 +59,7 @@ class StartCommand extends Command { type: 'number', default: 300 * 1000, }, - 'ignore-error': { + 'ignore-stderr': { description: 'whether ignore stderr when app starts', type: 'boolean', }, @@ -132,7 +132,7 @@ class StartCommand extends Command { this.logger.info('Starting %s application at %s', this.frameworkName, baseDir); // remove unused properties from stringify, alias had been remove by `removeAlias` - const ignoreKeys = [ '_', '$0', 'env', 'daemon', 'stdout', 'stderr', 'timeout', 'ignore-error' ]; + const ignoreKeys = [ '_', '$0', 'env', 'daemon', 'stdout', 'stderr', 'timeout', 'ignore-stderr' ]; const eggArgs = [ this.serverBin, stringify(argv, ignoreKeys), `--title=${argv.title}` ]; this.logger.info('Run node %s', eggArgs.join(' ')); @@ -181,7 +181,7 @@ class StartCommand extends Command { return name; } - * checkStatus({ stderr, timeout, 'ignore-error': ignoreError }) { + * checkStatus({ stderr, timeout, 'ignore-stderr': ignoreStdErr }) { let count = 0; let hasError = false; let isSuccess = true; @@ -214,7 +214,7 @@ class StartCommand extends Command { } catch (_) { // nothing } - isSuccess = ignoreError; + isSuccess = ignoreStdErr; this.logger.error('Start got error, see %s', stderr); } diff --git a/test/start.test.js b/test/start.test.js index f9d1575..9ef961e 100644 --- a/test/start.test.js +++ b/test/start.test.js @@ -468,13 +468,13 @@ describe('test/start.test.js', () => { .end(); }); - it('should status check fail `--ignore-error`, exit with 0', function* () { + it('should status check fail `--ignore-stderr`, exit with 0', function* () { mm(process.env, 'WAIT_TIME', 5000); mm(process.env, 'ERROR', 'error message'); const stderr = path.join(homePath, 'logs/master-stderr.log'); - yield coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1', '--ignore-error' ], { cwd }) + yield coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1', '--ignore-stderr' ], { cwd }) // .debug() .expect('stderr', /nodejs.Error: error message/) .expect('stderr', new RegExp(`Start got error, see ${stderr}`))