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. I also want all errors exposed through the service so I always have to throw and then have the main application promise catch it. Relies on: https://gerrit.wikimedia.org/r/#/c/151224 Change-Id: I6111cbc5e99d6bbfcb3293e8d274c6deb06a2775 --- M bin/mw-ocg-bundler M lib/index.js M lib/status.js M package.json M test/samples.js 5 files changed, 52 insertions(+), 50 deletions(-) Approvals: Cscott: Looks good to me, approved jenkins-bot: Verified diff --git a/bin/mw-ocg-bundler b/bin/mw-ocg-bundler index 8bf1aa6..3768f35 100755 --- a/bin/mw-ocg-bundler +++ b/bin/mw-ocg-bundler @@ -46,8 +46,6 @@ 'Print verbose progress information') .option('-D, --debug', 'Turn on debugging features (eg, full stack traces on exceptions)') - .option('--syslog', - 'Log errors using syslog (for production deployments)'); program.on('--help', function() { console.log(' If -o is omitted, creates bundle.zip'); console.log(' The -m option can be used instead of specifying titles'); @@ -71,24 +69,31 @@ require('../lib/retry-request').DEFAULT_TIMEOUT = program.timeout; } -var Syslog = program.syslog ? require('node-syslog') : { - init: function() { }, - log: function() { }, - close: function() { } -}; -Syslog.init(bundler.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 + }); + } } }; @@ -132,27 +137,21 @@ size: program.size, log: log }); -}).then(function(status) { - Syslog.close(); - if (status !== 0) { - // process.exit completes abruptly; sometimes there is - // outstanding "stuff" which node still needs to do - // (actually closing files, etc). In theory this shouldn't - // matter, but in practice it does. :( So don't call - // process.exit unless something seemed to have already - // gone wrong. - process.exit(status); - } -}); - -p.done(null, function(err) { - if (program.debug && err.stack) { - console.error(err.stack); - Syslog.log(Syslog.LOG_ERR, err.stack); +}).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 4f09e56..5dd2e10 100644 --- a/lib/index.js +++ b/lib/index.js @@ -33,17 +33,20 @@ version: json.version // version # for this package }; -// allow access to the metabook creation/repair functions +// Allow access to the metabook creation/repair functions module.exports.metabook = Metabook; -// returns a promise to create the given bundle +// Returns a promise to create the given bundle, which resolves with no +// value when the bundle is created successfully. +// The promise is rejected (with an object containing a non-zero `exitCode` +// property) if there is a problem creating the bundle. module.exports.bundle = function(metabook, options) { var stages = 5; if (options.compat) { stages += 1; /* fetchRevisions */ } var status = options.status = new StatusReporter(stages, function(msg) { if (options.log) { var file = msg.file ? (': ' + msg.file) : ''; - options.log('['+msg.percent.toFixed()+'%]', msg.status + file); + options.log('['+msg.percent.toFixed()+'%]', msg.message + file); } }); @@ -367,7 +370,7 @@ .then(createBundle) .then(function() { status.createStage(0, 'Done'); - return 0; + return; }, function(err) { // clean up if (cleanUpOutput) { @@ -375,11 +378,10 @@ rimraf.sync(options.output); } catch (e) { /* ignore */ } } - 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 0b34627..8812601 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,6 @@ "commander": "~2.2.0", "domino": "~1.0.17", "es6-shim": "~0.13.0", - "node-syslog": "~1.1.7", "prfun": "~1.0.0", "readable-stream": "~1.0.0", "request": "~2.37.0", diff --git a/test/samples.js b/test/samples.js index e29025d..b2a6426 100644 --- a/test/samples.js +++ b/test/samples.js @@ -35,8 +35,9 @@ console.log(time, util.format.apply(util, arguments)); } }); - }).then(function(statusCode) { - assert.equal(statusCode, 0); + }).then(function(_) { + // should resolve with no value + assert.equal(_, undefined); }).finally(function() { try { fs.unlinkSync(filename + '.zip'); -- To view, visit https://gerrit.wikimedia.org/r/151215 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6111cbc5e99d6bbfcb3293e8d274c6deb06a2775 Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/extensions/Collection/OfflineContentGenerator/bundler Gerrit-Branch: master Gerrit-Owner: Mwalker <[email protected]> Gerrit-Reviewer: Arlolra <[email protected]> Gerrit-Reviewer: Cscott <[email protected]> Gerrit-Reviewer: GWicke <[email protected]> Gerrit-Reviewer: Marcoil <[email protected]> Gerrit-Reviewer: MaxSem <[email protected]> Gerrit-Reviewer: Subramanya Sastry <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
