jenkins-bot has submitted this change and it was merged. Change subject: Use new log and status reporting framework ......................................................................
Use new log and status reporting framework Log through the service! Remove dependency on syslog. Expose all errors through the service by always rejecting the main application promise if anything goes wrong. Relies on: https://gerrit.wikimedia.org/r/#/c/151224 Change-Id: I330ebeba8c91d50ee7fd22998c3b94fc027e7c8f --- M bin/mw-ocg-texter M lib/index.js M lib/status.js M package.json M test/samples.js 5 files changed, 51 insertions(+), 40 deletions(-) Approvals: Cscott: Looks good to me, approved jenkins-bot: Verified diff --git a/bin/mw-ocg-texter b/bin/mw-ocg-texter index be37005..1b3b415 100755 --- a/bin/mw-ocg-texter +++ b/bin/mw-ocg-texter @@ -4,6 +4,7 @@ var program = require('commander'); var texter = require('../'); +var util = require('util'); program .version(texter.version) @@ -19,9 +20,7 @@ .option('-D, --debug', 'Turn on debugging features (eg, full stack traces on exceptions)') .option('-T, --temporary-directory <dir>', - 'Use <dir> for temporaries, not $TMPDIR or /tmp', null) - .option('--syslog', - 'Log errors using syslog (for production deployments)'); + 'Use <dir> for temporaries, not $TMPDIR or /tmp', null); program.parse(process.argv); @@ -36,24 +35,31 @@ var bundlefile = program.args[0]; -var Syslog = program.syslog ? require('node-syslog') : { - init: function() { }, - log: function() { }, - close: function() { } -}; -Syslog.init(texter.name, Syslog.LOG_PID | Syslog.LOG_ODELAY, Syslog.LOG_LOCAL0); - var log = function() { - // en/disable log messages here - if (program.verbose || program.debug) { - console.error.apply(console, arguments); - } try { - Syslog.log(Syslog.LOG_INFO, util.format.apply(this, arguments)); + // en/disable log messages here + if (program.verbose || program.debug) { + console.error.apply(console, arguments); + } + if (process.send) { + process.send({ + type: 'log', + level: 'info', + message: util.format.apply(null, arguments) + }); + } } catch (err) { // This should never happen! But don't try to convert arguments // toString() if it does, since that might fail too. - Syslog.log(Syslog.LOG_ERR, "Could not format message! "+err); + console.error("Could not format message!", err); + if (process.send) { + process.send({ + type: 'log', + level: 'error', + message: 'Could not format message! ' + err, + stack: err.stack + }); + } } }; @@ -67,17 +73,21 @@ log: log }; -texter.convert(options).then(function(status) { - Syslog.close(); - process.exit(status); -}, function(err) { - if (program.debug && err.stack) { - console.error(err.stack); - Syslog.log(Syslog.LOG_ERR, err.stack); +texter.convert(options).catch(function(err) { + var msg = { + type: 'log', + level: 'error' + }; + if ( err instanceof Error ) { + msg.message = err.message; + msg.stack = err.stack; } else { - console.error(err); - Syslog.log(Syslog.LOG_ERR, err); + msg.message = '' + err; } - Syslog.close(); - process.exit(1); + console.error( (program.debug && msg.stack) || msg.message ); + // process.send is sync, so we won't exit before this is sent (yay) + if (process.send) { + process.send(msg); + } + process.exit(err.exitCode || 1); }).done(); diff --git a/lib/index.js b/lib/index.js index c7de1c4..333e194 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1058,13 +1058,14 @@ }); }; -// Return a promise for an exit status (0 for success) after the bundle -// specified in the options has been converted. +// Return a promise which resolves with no value after the bundle +// specified in the options has been converted. The promise is +// rejected if there is a problem converting the bundle. var convert = function(options) { var status = options.status = new StatusReporter(2, function(msg) { if (options.log && options.output) { var file = msg.file ? (': ' + msg.file) : ''; - options.log('['+msg.percent.toFixed()+'%]', msg.status + file); + options.log('['+msg.percent.toFixed()+'%]', msg.message + file); } }); var metabook, builddir; @@ -1094,15 +1095,14 @@ return generateOutput(metabook, builddir, options); }).then(function() { status.createStage(0, 'Done'); - return 0; // success! + return; // success! }, function(err) { // xxx clean up? - if (options.debug) { - throw err; + // XXX use different values to distinguish failure types? + if (!err.exitCode) { + err.exitCode = 1; } - // xxx send this error to parent process? - console.error('Error:', err); - return 1; + throw err; }); }; diff --git a/lib/status.js b/lib/status.js index 3832366..272f282 100644 --- a/lib/status.js +++ b/lib/status.js @@ -17,7 +17,8 @@ /** Send one report with current status (internal function). */ StatusReporter.prototype._send = function() { var msg = { - status: this.message, + type: 'status', + message: this.message, file: this.file, percent: this.percentComplete }; diff --git a/package.json b/package.json index 5ad0451..7272756 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,6 @@ "domino": "~1.0.17", "es6-shim": "~0.13.0", "linewrap": "~0.2.1", - "node-syslog": "~1.1.7", "prfun": "~1.0.0", "readable-stream": "~1.0.0", "sqlite3": "~2.2.3", diff --git a/test/samples.js b/test/samples.js index 1ad7c79..dc9ada7 100644 --- a/test/samples.js +++ b/test/samples.js @@ -20,8 +20,9 @@ bundle: filename, output: filename + '.txt', log: function() { /* suppress logging */ } - }).then(function(statusCode) { - assert.equal(statusCode, 0); + }).then(function(_) { + // should resolve with no value + assert.equal(_, undefined); }).finally(function() { try { fs.unlinkSync(filename + '.txt'); -- To view, visit https://gerrit.wikimedia.org/r/159308 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I330ebeba8c91d50ee7fd22998c3b94fc027e7c8f Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/Collection/OfflineContentGenerator/text_renderer Gerrit-Branch: master Gerrit-Owner: Cscott <[email protected]> Gerrit-Reviewer: Cscott <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
