Skip to content

refactor: modify the directory of logDir #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 12, 2017
Merged

refactor: modify the directory of logDir #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4de9fad
refactor: modify the directory of logDir
popomore Oct 11, 2017
597c440
f
popomore Oct 11, 2017
346fd11
f
popomore Oct 11, 2017
bab863c
f
popomore Oct 11, 2017
275421b
f
popomore Oct 12, 2017
1a7805b
f
popomore Oct 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 41 additions & 38 deletions lib/cmd/start.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ class StartCommand extends Command {
description: 'whether run at background daemon mode',
type: 'boolean',
},
stdout: {
description: 'A file that stdout redirect to',
type: 'string',
},
stderr: {
description: 'A file that stderr redirect to',
type: 'string',
},
};
}

Expand All @@ -53,6 +61,8 @@ class StartCommand extends Command {

* run(context) {
const argv = Object.assign({}, context.argv);
const HOME = homedir();
const logDir = path.join(HOME, 'logs');

// egg-script start
// egg-script start ./server
Expand All @@ -63,46 +73,47 @@ class StartCommand extends Command {

const isDaemon = argv.daemon;

argv.framework = utils.getFrameworkPath({
argv.framework = yield this.getFrameworkPath({
framework: argv.framework,
baseDir,
});

const env = context.env;
env.PWD = baseDir;
env.HOME = homedir();
env.NODE_ENV = 'production';

// cli argv -> process.env.EGG_SERVER_ENV -> `undefined` then egg will use `prod`
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;
}

const pkgInfo = require(path.join(baseDir, 'package.json'));
const logDir = path.join(env.HOME, 'logs', pkgInfo.name);

argv.title = argv.title || `egg-server-${pkgInfo.name}`;

argv.stdout = argv.stdout || path.join(logDir, 'master-stdout.log');
argv.stderr = argv.stderr || path.join(logDir, 'master-stderr.log');

const env = context.env;
env.HOME = HOME;
env.NODE_ENV = 'production';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PWD 不传了？

看到下面 options 传了


// adjust env for win
let envPath = env.PATH || env.Path;
const envPath = env.PATH || env.Path;
if (envPath) {
// for nodeinstall
envPath = path.join(baseDir, 'node_modules/.bin') + path.delimiter + envPath;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个原来这样设置有啥用

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不是把 node_modules 里面的 node 加入到环境变量么?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

但是没加到 PATH

Copy link
Member

@atian25 atian25 Oct 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

哈... 写漏了。这块当时还在想怎么写测试

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个测不了，nyc 有 hack

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前想过一种，调用 nodeinstall 下载一个 node 版本，然后 app 里面 print 下，但麻烦就放弃了。

env.PATH = path.join(baseDir, 'node_modules/.bin') + path.delimiter + envPath;
}

// for alinode
env.ENABLE_NODE_LOG = 'YES';
env.NODE_LOG_DIR = env.NODE_LOG_DIR || path.join(logDir, 'alinode');
yield mkdirp(env.NODE_LOG_DIR);

// cli argv -> process.env.EGG_SERVER_ENV -> `undefined` then egg will use `prod`
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;

const options = {
cwd: baseDir,
execArgv: context.execArgv,
env,
stdio: 'inherit',
Expand All @@ -117,11 +128,11 @@ class StartCommand extends Command {
// whether run in the background.
if (isDaemon) {
this.logger.info(`save log file to ${logDir}`);
const { stdout, stderr } = yield getRotatelog(logDir);
const [ stdout, stderr ] = yield [ getRotatelog(argv.stdout), getRotatelog(argv.stderr) ];
options.stdio = [ 'ignore', stdout, stderr, 'ipc' ];
options.detached = true;

const child = spawn('node', eggArgs, options);
const child = this.child = spawn('node', eggArgs, options);
child.on('message', msg => {
if (msg && msg.action === 'egg-ready') {
this.logger.info(`egg started on ${msg.data.address}`);
Expand All @@ -135,32 +146,24 @@ class StartCommand extends Command {
this.helper.spawn('node', eggArgs, options);
}
}
}

function* getRotatelog(logDir) {
const stdoutPath = path.join(logDir, 'master-stdout.log');
const stderrPath = path.join(logDir, 'master-stderr.log');
* getFrameworkPath(params) {
return utils.getFrameworkPath(params);
}

// format style: .20150602.193100
const timestamp = moment().format('.YYYYMMDD.HHmmss');
}

yield mkdirp(logDir);
function* getRotatelog(logfile) {
yield mkdirp(path.dirname(logfile));

/* istanbul ignore else */
if (yield fs.exists(stdoutPath)) {
if (yield fs.exists(logfile)) {
// format style: .20150602.193100
const timestamp = moment().format('.YYYYMMDD.HHmmss');
// Note: rename last log to next start time, not when last log file created
yield fs.rename(stdoutPath, stdoutPath + timestamp);
}

/* istanbul ignore else */
if (yield fs.exists(stderrPath)) {
yield fs.rename(stderrPath, stderrPath + timestamp);
yield fs.rename(logfile, logfile + timestamp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里其实如果能重命名为上次启动的时间，而不是本次的时间会更好一点，避免误导。
但好像 fs 拿文件的 create time 不准？

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

标记为重启时间也还行

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

只是感觉会误导的，一般不都是文件名 + 启动时间 么？ 当时想改的，但没找到获取时间的地方。

不过这里面应该也没什么太多有价值的 log。

}

return yield {
stdout: fs.open(stdoutPath, 'a'),
stderr: fs.open(stderrPath, 'a'),
};
return yield fs.open(logfile, 'a');
}

module.exports = StartCommand;
7 changes: 4 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@
"node": ">=6.0.0"
},
"scripts": {
"test": "npm run lint -- --fix && npm run test-local",
"pkgfiles": "egg-bin pkgfiles",
"test": "npm run lint -- --fix && npm run pkgfiles && npm run test-local",
"test-local": "egg-bin test",
"cov": "egg-bin cov",
"lint": "eslint .",
"ci": "egg-bin pkgfiles --check && npm run lint && npm run cov",
"ci": "npm run pkgfiles -- --check && npm run lint && npm run cov",
"autod": "autod"
},
"files": [
Expand All @@ -57,4 +58,4 @@
},
"author": "TZ <atian25@qq.com>",
"license": "MIT"
}
}
4 changes: 4 additions & 0 deletions test/fixtures/example/app/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ module.exports = app => {
app.get('/env', function* () {
this.body = app.config.env + ', ' + app.config.pre;
});

app.get('/path', function* () {
this.body = process.env.PATH;
});
};
50 changes: 46 additions & 4 deletions test/start.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,24 @@ const sleep = require('mz-modules/sleep');
const rimraf = require('mz-modules/rimraf');
const mkdirp = require('mz-modules/mkdirp');
const coffee = require('coffee');
const homedir = require('node-homedir');
const httpclient = require('urllib');
const mm = require('mm');
const utils = require('./utils');

describe('test/start.test.js', () => {
const eggBin = require.resolve('../bin/egg-scripts.js');
const fixturePath = path.join(__dirname, 'fixtures/example');
const homePath = homedir();
const logDir = path.join(homePath, 'logs/example');
const homePath = path.join(__dirname, 'fixtures/home');
const logDir = path.join(homePath, 'logs');
const waitTime = '10s';

before(function* () {
yield mkdirp(homePath);
});
after(function* () {
yield rimraf(homePath);
});
beforeEach(() => mm(process.env, 'MOCK_HOME_DIR', homePath));
afterEach(() => mm.restore);

describe('start without daemon', () => {
Expand Down Expand Up @@ -262,8 +268,43 @@ describe('test/start.test.js', () => {
assert(app.stdout.includes('## EGG_SERVER_ENV is not pass'));
assert(app.stdout.includes('## CUSTOM_ENV: pre'));
assert(app.stdout.match(/custom-framework started on http:\/\/127\.0\.0\.1:7001/));
const result = yield httpclient.request('http://127.0.0.1:7001/env');
let result = yield httpclient.request('http://127.0.0.1:7001/env');
assert(result.data.toString() === 'pre, true');
result = yield httpclient.request('http://127.0.0.1:7001/path');
assert(result.data.toString().match(new RegExp(`^${fixturePath}/node_modules/.bin${path.delimiter}`)));
});
});

describe('--stdout --stderr', () => {
let app;

before(function* () {
yield utils.cleanup(fixturePath);
yield rimraf(logDir);
yield mkdirp(logDir);
});

after(function* () {
app.proc.kill('SIGTERM');
yield utils.cleanup(fixturePath);
yield rimraf(path.join(fixturePath, 'stdout.log'));
yield rimraf(path.join(fixturePath, 'stderr.log'));
});

it('should start', function* () {
const stdout = path.join(fixturePath, 'stdout.log');
const stderr = path.join(fixturePath, 'stderr.log');
app = coffee.fork(eggBin, [ 'start', '--workers=1', '--daemon', `--stdout=${stdout}`, `--stderr=${stderr}`, fixturePath ]);
// app.debug();
app.expect('code', 0);

yield sleep(waitTime);

let content = yield fs.readFile(stdout, 'utf-8');
assert(content.match(/custom-framework started on http:\/\/127\.0\.0\.1:7001/));

content = yield fs.readFile(stderr, 'utf-8');
assert(content === '');
});
});

Expand Down Expand Up @@ -337,4 +378,5 @@ describe('test/start.test.js', () => {
assert(result.data.toString() === 'hi, egg');
});
});

});
16 changes: 13 additions & 3 deletions test/stop.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,28 @@ const assert = require('assert');
const fs = require('mz/fs');
const sleep = require('mz-modules/sleep');
const rimraf = require('mz-modules/rimraf');
const mkdirp = require('mz-modules/mkdirp');
const coffee = require('coffee');
const homedir = require('node-homedir');
const httpclient = require('urllib');
const mm = require('mm');
const utils = require('./utils');

describe('test/stop.test.js', () => {
const eggBin = require.resolve('../bin/egg-scripts.js');
const fixturePath = path.join(__dirname, 'fixtures/example');
const homePath = homedir();
const logDir = path.join(homePath, 'logs/example');
const homePath = path.join(__dirname, 'fixtures/home');
const logDir = path.join(homePath, 'logs');
const waitTime = '10s';

before(function* () {
yield mkdirp(homePath);
});
after(function* () {
yield rimraf(homePath);
});
beforeEach(() => mm(process.env, 'MOCK_HOME_DIR', homePath));
afterEach(() => mm.restore);

describe('stop without daemon', () => {
let app;
let killer;
Expand Down