Skip to content

Commit

Permalink
feat: close return promise
Browse files Browse the repository at this point in the history
logger should close stream when app close
  • Loading branch information
popomore committed Oct 24, 2016
1 parent 03d70e1 commit c43a578
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 48 deletions.
7 changes: 5 additions & 2 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,11 @@ class Agent extends EggApplication {
}

close() {
process.removeListener('uncaughtException', this._uncaughtExceptionHandler);
super.close();
return super
.close()
.then(() => {
process.removeListener('uncaughtException', this._uncaughtExceptionHandler);
});
}

}
Expand Down
14 changes: 10 additions & 4 deletions lib/egg.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,15 +225,21 @@ class EggApplication extends EggCore {
this.ready(() => clearTimeout(startTimeoutTimer));
}


/**
* 关闭 app 上的所有事件监听
* @public
* @return {Promise} promise
*/
close() {
super.close();
this.messenger.close();
process.removeListener('unhandledRejection', this._unhandledRejectionHandler);
return super
.close()
.then(() => {
for (const logger of this.loggers.values()) {
logger.end();
}
this.messenger.close();
process.removeListener('unhandledRejection', this._unhandledRejectionHandler);
});
}
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"delegates": "^1.0.0",
"egg-cluster": "^0.1.0",
"egg-cookies": "^1.0.0",
"egg-core": "^0.2.1",
"egg-core": "~0.4.0",
"egg-cors": "^0.0.2",
"egg-development": "^1.0.1",
"egg-i18n": "^1.0.2",
Expand Down
1 change: 1 addition & 0 deletions test/app/extend/context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ describe('test/app/extend/context.test.js', () => {
it('should render template', () => {
return request(app.callback())
.get('/')
.expect(res => console.log(res.text))
.expect(200)
.expect('name=index.html, a=111, b=b, c=testHelper');
});
Expand Down
4 changes: 3 additions & 1 deletion test/app/middleware/site_file.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const should = require('should');
const request = require('supertest');
const assert = require('assert');
const utils = require('../../utils');

describe('test/app/middleware/site_file.test.js', () => {
Expand Down Expand Up @@ -47,7 +48,8 @@ describe('test/app/middleware/site_file.test.js', () => {
return request(app.callback())
.head('/robots.txt')
.expect('content-length', '72')
.expect('') // body must be empty for HEAD
// body must be empty for HEAD
.expect(res => assert.equal(res.text, undefined))
.expect(200);
});

Expand Down
File renamed without changes.
4 changes: 2 additions & 2 deletions test/lib/agent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ describe('test/lib/agent.test.js', () => {
});

describe('close()', () => {
it('should close all listeners', function() {
it('should close all listeners', function* () {
const baseDir = path.join(__dirname, '../fixtures/apps/demo');
const agent = new Agent({
baseDir,
});
process.listeners('unhandledRejection')
.indexOf(agent._unhandledRejectionHandler).should.not.equal(-1);
agent.close();
yield agent.close();
process.listeners('unhandledRejection')
.indexOf(agent._unhandledRejectionHandler).should.equal(-1);
});
Expand Down
4 changes: 2 additions & 2 deletions test/lib/application.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ describe('test/lib/application.test.js', () => {
});

describe('close()', () => {
it('should close all listeners', () => {
it('should close all listeners', function* () {
const baseDir = path.join(__dirname, '../fixtures/apps/demo');
const application = new Application({
baseDir,
});
process.listeners('unhandledRejection')
.indexOf(application._unhandledRejectionHandler).should.not.equal(-1);
application.close();
yield application.close();
process.listeners('unhandledRejection')
.indexOf(application._unhandledRejectionHandler).should.equal(-1);
});
Expand Down
10 changes: 9 additions & 1 deletion test/lib/core/loader/load_plugin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const utils = require('../../../utils');

const EGG_BASE = path.join(__dirname, '../../../../');

describe('test/lib/core/loader.test.js', () => {
describe('test/lib/core/loader/load_plugin.test.js', () => {
let app;
const logger = console;
before(() => {
Expand All @@ -32,20 +32,23 @@ describe('test/lib/core/loader.test.js', () => {
dep: [],
env: [],
path: path.join(baseDir, 'node_modules/b'),
from: path.join(baseDir, 'config/plugin.js'),
});
appLoader.plugins.c.should.eql({
enable: true,
name: 'c',
dep: [],
env: [],
path: path.join(baseDir, 'node_modules/c'),
from: path.join(baseDir, 'config/plugin.js'),
});
appLoader.plugins.e.should.eql({
enable: true,
name: 'e',
dep: [ 'f' ],
env: [],
path: path.join(baseDir, 'plugins/e'),
from: path.join(baseDir, 'config/plugin.js'),
});
appLoader.plugins.onerror.path.should.equal(fs.realpathSync(path.join(EGG_BASE, 'node_modules/egg-onerror')));
appLoader.plugins.onerror.package.should.equal('egg-onerror');
Expand All @@ -69,6 +72,7 @@ describe('test/lib/core/loader.test.js', () => {
env: [],
package: 'rds',
path: path.join(baseDir, 'node_modules/rds'),
from: path.join(baseDir, 'config/plugin.js'),
});
});

Expand All @@ -87,6 +91,7 @@ describe('test/lib/core/loader.test.js', () => {
dep: [],
env: [],
path: path.join(baseDir, 'node_modules/d'),
from: path.join(baseDir, 'config/plugin.js'),
});
should.not.exists(appLoader.plugins.d);
});
Expand All @@ -105,6 +110,7 @@ describe('test/lib/core/loader.test.js', () => {
dep: [ 'f' ],
env: [],
path: path.join(baseDir, 'plugins/g'),
from: path.join(baseDir, 'config/plugin.js'),
});
});

Expand Down Expand Up @@ -152,6 +158,7 @@ describe('test/lib/core/loader.test.js', () => {
dep: [],
env: [ 'unittest' ],
path: path.join(baseDir, 'node_modules/d'),
from: path.join(baseDir, 'config/plugin.js'),
});
appLoader.plugins.foo.should.eql({
enable: true,
Expand Down Expand Up @@ -277,6 +284,7 @@ describe('test/lib/core/loader.test.js', () => {
dep: [ 'd1' ],
env: [ 'local', 'prod' ],
path: path.join(baseDir, 'node_modules/a1'),
from: path.join(baseDir, 'config/plugin.js'),
});
});
});
60 changes: 25 additions & 35 deletions test/lib/core/logger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,74 +11,76 @@ const utils = require('../../utils');
const Agent = require('../../..').Agent;

describe('test/lib/core/logger.test.js', () => {

let app;
afterEach(mm.restore);
afterEach(() => app.close());

it('should got right default config on prod env', () => {
mm.env('prod');
mm(process.env, 'EGG_LOG', '');
mm(process.env, 'HOME', utils.getFilepath('apps/mock-production-app/config'));
const app = utils.app('apps/mock-production-app');
app = utils.app('apps/mock-production-app');
// 生产环境默认 _level = info
app.logger.get('file').options.level.should.equal(Logger.INFO);
// stdout 默认 INFO
app.logger.get('console').options.level.should.equal(Logger.INFO);
app.coreLogger.get('file').options.level.should.equal(Logger.INFO);
app.coreLogger.get('console').options.level.should.equal(Logger.INFO);
app.close();
return app.ready();
});

it('should got right level on local env', () => {
mm.env('local');
mm(process.env, 'EGG_LOG', '');
const app = utils.app('apps/mock-dev-app');
app = utils.app('apps/mock-dev-app');

app.logger.get('file').options.level.should.equal(Logger.DEBUG);
app.logger.get('console').options.level.should.equal(Logger.INFO);
app.coreLogger.get('file').options.level.should.equal(Logger.DEBUG);
app.coreLogger.get('console').options.level.should.equal(Logger.WARN);
app.close();
return app.ready();
});

it('should set EGG_LOG level on local env', () => {
mm.env('local');
mm(process.env, 'EGG_LOG', 'ERROR');
const app = utils.app('apps/mock-dev-app');
app = utils.app('apps/mock-dev-app');
app.logger.get('file').options.level.should.equal(Logger.DEBUG);
app.logger.get('console').options.level.should.equal(Logger.ERROR);
app.coreLogger.get('file').options.level.should.equal(Logger.DEBUG);
app.coreLogger.get('console').options.level.should.equal(Logger.ERROR);
app.close();
return app.ready();
});

it('should got right config on unittest env', () => {
mm.env('unittest');
mm(process.env, 'EGG_LOG', '');
const app = utils.app('apps/mock-dev-app');
app = utils.app('apps/mock-dev-app');
app.logger.get('file').options.level.should.equal(Logger.INFO);
app.logger.get('console').options.level.should.equal(Logger.WARN);
app.coreLogger.get('file').options.level.should.equal(Logger.INFO);
app.coreLogger.get('console').options.level.should.equal(Logger.WARN);
app.close();
return app.ready();
});

it('should set log.consoleLevel to env.EGG_LOG', () => {
mm(process.env, 'EGG_LOG', 'ERROR');
const app = utils.app('apps/mock-dev-app');
app = utils.app('apps/mock-dev-app');
app.logger.get('file').options.level.should.equal(Logger.INFO);
app.logger.get('console').options.level.should.equal(Logger.ERROR);
app.close();
return app.ready();
});

it('log buffer disable cache on local and unittest env', done => {
mm(process.env, 'EGG_LOG', 'NONE');
const app = utils.app('apps/nobuffer-logger');
app = utils.app('apps/nobuffer-logger');
app.ready(() => {
const ctx = app.mockContext();
const logfile = path.join(app.config.logger.dir, 'common-error.log');
// app.config.logger.buffer.should.equal(false);
ctx.logger.error(new Error('mock nobuffer error'));
setTimeout(() => {
app.close();
fs.readFileSync(logfile, 'utf8').should.containEql('nodejs.Error: mock nobuffer error\n');
done();
}, 1000);
Expand All @@ -89,14 +91,13 @@ describe('test/lib/core/logger.test.js', () => {
mm(process.env, 'EGG_LOG', 'none');
mm.env('prod');
mm(process.env, 'HOME', utils.getFilepath('apps/mock-production-app/config'));
const app = utils.app('apps/mock-production-app');
app = utils.app('apps/mock-production-app');
app.ready(() => {
const ctx = app.mockContext();
const logfile = path.join(app.config.logger.dir, 'common-error.log');
// app.config.logger.buffer.should.equal(true);
ctx.logger.error(new Error('mock enable buffer error'));
setTimeout(() => {
app.close();
fs.readFileSync(logfile, 'utf8').should.containEql('');
done();
}, 1000);
Expand All @@ -106,13 +107,12 @@ describe('test/lib/core/logger.test.js', () => {
it('output .json format log', done => {
mm(process.env, 'EGG_LOG', 'none');
mm.env('local');
const app = utils.app('apps/logger-output-json');
app = utils.app('apps/logger-output-json');
app.ready(() => {
const ctx = app.mockContext();
const logfile = path.join(app.config.logger.dir, 'logger-output-json-web.json.log');
ctx.logger.info('json format');
setTimeout(() => {
app.close();
fs.existsSync(logfile).should.be.true;
fs.readFileSync(logfile, 'utf8').should.containEql('"message":"json format"');
done();
Expand All @@ -122,7 +122,7 @@ describe('test/lib/core/logger.test.js', () => {

it('dont output to console after app ready', done => {
mm.env('default');
const app = utils.cluster('apps/logger');
app = utils.cluster('apps/logger');
app
.debug(false)
.coverage(false)
Expand All @@ -131,16 +131,12 @@ describe('test/lib/core/logger.test.js', () => {
.notExpect('stdout', /app info after ready/)
.expect('stderr', /nodejs.Error: agent error/)
.expect('stderr', /nodejs.Error: app error/)
.end(err => {
app.close();
should.not.exists(err);
done();
});
.end(done);
});

it('should still output to console after app ready on local env', done => {
mm.env('local');
const app = utils.cluster('apps/logger');
app = utils.cluster('apps/logger');
app
// .debug()
.coverage(false)
Expand All @@ -149,24 +145,19 @@ describe('test/lib/core/logger.test.js', () => {
.expect('stdout', /app info after ready/)
.expect('stderr', /nodejs.Error: agent error/)
.expect('stderr', /nodejs.Error: app error/)
.end(err => {
app.close();
should.not.exists(err);
done();
});
.end(done);
});

it('agent and app error should output to common-error.log', done => {
const baseDir = utils.getFilepath('apps/logger');
mm.env('default');
mm(process.env, 'EGG_LOG', 'none');
mm(process.env, 'HOME', baseDir);
const app = utils.cluster('apps/logger');
app = utils.cluster('apps/logger');
app
// .debug()
.coverage(false)
.end(err => {
app.close();
should.not.exists(err);
const content = fs.readFileSync(path.join(baseDir, 'logs/logger/common-error.log'), 'utf8');
content.should.containEql('nodejs.Error: agent error');
Expand All @@ -176,15 +167,14 @@ describe('test/lib/core/logger.test.js', () => {
});

it('all loggers error should redirect to errorLogger', done => {
const app = utils.app('apps/logger');
app = utils.app('apps/logger');
app.ready(() => {
app.logger.error(new Error('logger error'));
app.coreLogger.error(new Error('coreLogger error'));
app.loggers.errorLogger.error(new Error('errorLogger error'));
app.loggers.customLogger.error(new Error('customLogger error'));

setTimeout(() => {
app.close();
const content = fs.readFileSync(path.join(app.baseDir, 'logs/logger/common-error.log'), 'utf8');
content.should.containEql('nodejs.Error: logger error');
content.should.containEql('nodejs.Error: coreLogger error');
Expand All @@ -196,11 +186,11 @@ describe('test/lib/core/logger.test.js', () => {
});

it('agent\'s logger is same as coreLogger', done => {
const agent = new Agent({
app = new Agent({
baseDir: utils.getFilepath('apps/logger'),
});
agent.logger.options.file.should.equal(agent.coreLogger.options.file);
agent.ready(done);
app.logger.options.file.should.equal(app.coreLogger.options.file);
app.ready(done);
});

describe.skip('logger.reload()', () => {
Expand Down

0 comments on commit c43a578

Please sign in to comment.