Cscott has uploaded a new change for review. https://gerrit.wikimedia.org/r/159308
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 4 files changed, 48 insertions(+), 38 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Collection/OfflineContentGenerator/text_renderer refs/changes/08/159308/1 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", -- To view, visit https://gerrit.wikimedia.org/r/159308 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I330ebeba8c91d50ee7fd22998c3b94fc027e7c8f Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Collection/OfflineContentGenerator/text_renderer Gerrit-Branch: master Gerrit-Owner: Cscott <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
