Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Commit 28d9240

Browse files
davek-at-ibmdavek-at-ibm
authored
[Master] ensure logging files are flushed for CLI (#4480)
Signed-off-by: Dave Kelsey <d_kelsey@uk.ibm.com>
1 parent 11c7783 commit 28d9240

4 files changed

Lines changed: 115 additions & 11 deletions

File tree

‎packages/composer-cli/cli.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const cmdUtil = require('./lib/cmds/utils/cmdutils');
2121
const yargs = require('yargs');
2222
const chalk = require('chalk');
2323
const version = 'v' +require('./package.json').version;
24+
const Logger = require('composer-common').Logger;
25+
const LOG = Logger.getLog('CLI');
26+
LOG.info('CLI', 'CLI Launched with', process.argv);
2427

2528
let results = yargs
2629
.commandDir('./lib/cmds')
@@ -41,11 +44,11 @@ if (typeof(results.thePromise) !== 'undefined'){
4144
if (!results.quiet) {
4245
cmdUtil.log(chalk.green('\nCommand succeeded\n'));
4346
}
44-
process.exit(0);
47+
Logger.flushLogFileAndExit(0);
4548
}).catch((error) => {
4649
cmdUtil.log(error+chalk.red('\nCommand failed\n'));
47-
process.exit(1);
50+
Logger.flushLogFileAndExit(1);
4851
});
4952
} else {
50-
process.exit(0);
53+
Logger.flushLogFileAndExit(0);
5154
}

‎packages/composer-cli/test/cli.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ chai.should();
2020
chai.use(require('chai-things'));
2121
chai.use(require('chai-as-promised'));
2222
const path = require('path');
23+
const Logger = require('composer-common').Logger;
2324

2425
const yargs = require('yargs');
2526
const chalk = require('chalk');
@@ -66,7 +67,7 @@ describe('composer cli', () => {
6667
sandbox.stub(yargs, 'alias').returns(yargs);
6768
sandbox.stub(yargs, 'version').returns(yargs);
6869

69-
sandbox.stub(process, 'exit');
70+
sandbox.stub(Logger, 'flushLogFileAndExit');
7071
sandbox.stub(chalk, 'green');
7172
sandbox.stub(chalk, 'red');
7273
});
@@ -92,27 +93,27 @@ describe('composer cli', () => {
9293
sinon.assert.calledOnce(yargs.alias);
9394
sinon.assert.calledOnce(yargs.version);
9495
sinon.assert.calledOnce(yargs.describe);
95-
sinon.assert.calledWith(process.exit, 0);
96+
sinon.assert.calledWith(Logger.flushLogFileAndExit, 0);
9697

9798
});
9899

99100
it('Should handle resolved promise', () => {
100101
sandbox.stub(yargs, 'describe').returns({ argv: {thePromise: new fakePromise()} });
101102
delete require.cache[path.resolve(__dirname, '../cli.js')];
102103
require('../cli.js');
103-
sinon.assert.calledWith(process.exit, 0);
104+
sinon.assert.calledWith(Logger.flushLogFileAndExit, 0);
104105
});
105106
it('Should handle resolved promise, with the quiet setting', () => {
106107
sandbox.stub(yargs, 'describe').returns({ argv: {thePromise: new fakePromise(),quiet:true} });
107108
delete require.cache[path.resolve(__dirname, '../cli.js')];
108109
require('../cli.js');
109-
sinon.assert.calledWith(process.exit, 0);
110+
sinon.assert.calledWith(Logger.flushLogFileAndExit, 0);
110111
});
111112
it('Should handle rejected promise', () => {
112113
sandbox.stub(yargs, 'describe').returns({ argv: {thePromise: new fakePromise()} });
113114
delete require.cache[path.resolve(__dirname, '../cli.js')];
114115
require('../cli.js');
115-
sinon.assert.calledWith(process.exit, 1);
116+
sinon.assert.calledWith(Logger.flushLogFileAndExit, 1);
116117
});
117118

118119
});

‎packages/composer-common/lib/log/logger.js‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,26 @@ class Logger {
431431
return _config;
432432
}
433433

434+
/**
435+
* flush out the standard composer log file and exit. This is only of use to CLI
436+
* applications which will log to the file system.
437+
* @param {*} err the exit value
438+
*/
439+
static flushLogFileAndExit(err) {
440+
const fileTransport = _logger && _logger.transports ? _logger.transports['debug-file'] : null;
441+
if (fileTransport && fileTransport._stream) {
442+
fileTransport.on('flush', () => {
443+
process.exit(err);
444+
});
445+
// calling close on the logger sometimes hung the cli
446+
// flush could fail if there was no stream, but appears to be more reliable.
447+
fileTransport.flush();
448+
} else {
449+
process.exit(err);
450+
}
451+
452+
}
453+
434454
/**
435455
* return the log configuration that is in force, note that this method just returns the information
436456
* it does create, modify or delete it
@@ -667,9 +687,7 @@ class Logger {
667687
const loggerToUse = localConfig.logger;
668688
let myLogger;
669689
try {
670-
// const mod = 'config';
671690
const req = require;
672-
// const config = req(mod);
673691
myLogger = req(loggerToUse);
674692
} catch (e) {
675693
// Print the error to the console and just use the null logger instead.

‎packages/composer-common/test/log/logger.js‎

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ describe('Logger', () => {
459459

460460
});
461461

462-
describe('#getLoggerCfg', () => {
462+
describe('#processLoggerConfig', () => {
463463
let sandbox;
464464

465465
beforeEach(()=>{
@@ -869,6 +869,88 @@ describe('Logger', () => {
869869

870870
});
871871

872+
describe('#flushLogFileAndExit', () => {
873+
let sandbox;
874+
875+
beforeEach(()=>{
876+
Logger.__reset();
877+
sandbox = sinon.sandbox.create();
878+
});
879+
880+
afterEach(()=>{
881+
Logger.__reset();
882+
sandbox.restore();
883+
});
884+
885+
it('should just exit with error code if no transports defined', () => {
886+
sandbox.stub(process, 'exit');
887+
let stubLogger = {
888+
};
889+
890+
Logger.setFunctionalLogger(stubLogger);
891+
Logger.flushLogFileAndExit(99);
892+
sinon.assert.calledOnce(process.exit);
893+
sinon.assert.calledWith(process.exit, 99);
894+
});
895+
896+
it('should just exit with error code if no file logger transport defined', () => {
897+
sandbox.stub(process, 'exit');
898+
let stubTransport = {
899+
on: sinon.stub(),
900+
flush: sinon.stub()
901+
};
902+
let stubLogger = {
903+
transports: {'info-file': stubTransport}
904+
};
905+
906+
Logger.setFunctionalLogger(stubLogger);
907+
Logger.flushLogFileAndExit(0);
908+
sinon.assert.calledOnce(process.exit);
909+
sinon.assert.calledWith(process.exit, 0);
910+
sinon.assert.notCalled(stubTransport.on);
911+
sinon.assert.notCalled(stubTransport.flush);
912+
});
913+
914+
it('should just exit with error code if file logger transport defined, but no stream exists', () => {
915+
sandbox.stub(process, 'exit');
916+
let stubTransport = {
917+
on: sinon.stub(),
918+
flush: sinon.stub()
919+
};
920+
let stubLogger = {
921+
transports: {'debug-file': stubTransport}
922+
};
923+
924+
Logger.setFunctionalLogger(stubLogger);
925+
Logger.flushLogFileAndExit(0);
926+
sinon.assert.calledOnce(process.exit);
927+
sinon.assert.calledWith(process.exit, 0);
928+
sinon.assert.notCalled(stubTransport.on);
929+
sinon.assert.notCalled(stubTransport.flush);
930+
});
931+
932+
933+
it('should register a flush handler and exit only when that exit handler is called if file logger transport defined', () => {
934+
sandbox.stub(process, 'exit');
935+
let stubTransport = {
936+
on: sinon.stub().yields(),
937+
flush: sinon.stub(),
938+
_stream: {}
939+
};
940+
let stubLogger = {
941+
close: sinon.stub(),
942+
transports: {'debug-file': stubTransport}
943+
};
944+
945+
Logger.setFunctionalLogger(stubLogger);
946+
Logger.flushLogFileAndExit(77);
947+
sinon.assert.calledOnce(process.exit);
948+
sinon.assert.calledWith(process.exit, 77);
949+
sinon.assert.calledOnce(stubTransport.flush);
950+
sinon.assert.calledOnce(stubTransport.on);
951+
});
952+
});
953+
872954
describe('#invokeAllLevels', ()=>{
873955
let logger = {
874956
debug: sinon.stub(),

0 commit comments

Comments
 (0)