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: I00b1479404441063132c57f3ffe1c37f80658938
---
M bin/mw-ocg-latexer
M lib/index.js
M lib/status.js
M package.json
M test/samples.js
5 files changed, 46 insertions(+), 43 deletions(-)

Approvals:
  Cscott: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/bin/mw-ocg-latexer b/bin/mw-ocg-latexer
index 4e4c1f6..ac65465 100755
--- a/bin/mw-ocg-latexer
+++ b/bin/mw-ocg-latexer
@@ -31,9 +31,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);
 
@@ -48,24 +46,31 @@
 
 var bundlefile = program.args[0];
 
-var Syslog = program.syslog ? require('node-syslog') : {
-       init: function() { },
-       log: function() { },
-       close: function() { }
-};
-Syslog.init(latexer.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
+                       });
+               }
        }
 };
 
@@ -85,17 +90,21 @@
        options.toc = !/^(no|false|off)$/i.test(program.toc);
 }
 
-latexer.convert(options).then(function(status) {
-       Syslog.close();
-       process.exit(status);
-}, function(err) {
-       if ((program.debug || program.syslog) && err.stack) {
-               console.error(err.stack);
-               Syslog.log(Syslog.LOG_ERR, JSON.stringify(err.stack));
+latexer.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 a73b4dc..ab17b40 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -1780,14 +1780,15 @@
  *
  * Convert a bundle to LaTeX and/or a PDF, respecting the given `options`.
  *
- * Return a promise for an exit status (0 for success) after the bundle
- * specified in the options has been converted.
+ * Return a promise which is resolved with no value after the bundle
+ * specified in the options has been converted.  If there is a problem
+ * during the conversion, the promise is rejected.
  */
 var convert = function(options) {
        var status = options.status = new StatusReporter(4, 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);
                }
        });
        var metabook, builddir, imagemap;
@@ -1825,15 +1826,7 @@
                return compileLatex(builddir, options);
        }).then(function() {
                status.createStage(0, 'Done');
-               return 0; // success!
-       }, function(err) {
-               // xxx clean up?
-               if (options.debug) {
-                       throw err;
-               }
-               // xxx send this error to parent process, if there is one?
-               console.error('Error:', err);
-               return 1;
+               return; // success!
        });
 };
 
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 64913b8..e210403 100644
--- a/package.json
+++ b/package.json
@@ -26,7 +26,6 @@
     "es6-shim": "~0.13.0",
     "gammalatex": "cscott/gammalatex#race-be-gone",
     "icu-bidi": "~0.1.2",
-    "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 56e37a4..72b464c 100644
--- a/test/samples.js
+++ b/test/samples.js
@@ -47,8 +47,9 @@
                                        latex: !hasXeLaTeX,
                                        skipJpegtran: !hasJpegtran,
                                        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 + '.' + 
dest);

-- 
To view, visit https://gerrit.wikimedia.org/r/151222
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I00b1479404441063132c57f3ffe1c37f80658938
Gerrit-PatchSet: 5
Gerrit-Project: 
mediawiki/extensions/Collection/OfflineContentGenerator/latex_renderer
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

Reply via email to