Physikerwelt has submitted this change and it was merged.

Change subject: Update to service-template-node v0.2.3
......................................................................


Update to service-template-node v0.2.3

- Log the error stack only for 500 errors
- Better configuration params handling
- Update dependencies

Change-Id: I252f2a71929b6919d4f577d365517cae327a8c60
---
M .travis.yml
M app.js
M config.dev.yaml
M doc/deployment.md
M lib/util.js
M package.json
M test/features/app/app.js
M test/utils/server.js
8 files changed, 109 insertions(+), 72 deletions(-)

Approvals:
  Physikerwelt: Verified; Looks good to me, approved



diff --git a/.travis.yml b/.travis.yml
index cd47495..b47877b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,8 @@
 language: node_js
+
 node_js:
-    - "4.2.4"
+  - "4.2.4"
+  - "5"
 
 script: npm run-script coverage && (npm run-script coveralls || exit 0)
 
diff --git a/app.js b/app.js
index e7f9f99..376f9fa 100644
--- a/app.js
+++ b/app.js
@@ -37,9 +37,9 @@
     // ensure some sane defaults
     if(!app.conf.port) { app.conf.port = 10042; }
     if(!app.conf.interface) { app.conf.interface = '0.0.0.0'; }
-    if(!app.conf.compression_level) { app.conf.compression_level = 3; }
+    if(app.conf.compression_level === undefined) { app.conf.compression_level 
= 3; }
     if(app.conf.cors === undefined) { app.conf.cors = '*'; }
-    if(!app.conf.csp) {
+    if(app.conf.csp === undefined) {
         app.conf.csp =
             "default-src 'self'; object-src 'none'; media-src *; img-src *; 
style-src *; frame-ancestors 'self'";
     }
@@ -96,6 +96,25 @@
         app.conf.spec.paths = {};
     }
 
+    // set the CORS and CSP headers
+    app.all('*', function(req, res, next) {
+        if(app.conf.cors !== false) {
+            res.header('access-control-allow-origin', app.conf.cors);
+            res.header('access-control-allow-headers', 'accept, 
x-requested-with, content-type');
+            res.header('access-control-expose-headers', 'etag');
+        }
+        if(app.conf.csp !== false) {
+            res.header('x-xss-protection', '1; mode=block');
+            res.header('x-content-type-options', 'nosniff');
+            res.header('x-frame-options', 'SAMEORIGIN');
+            res.header('content-security-policy', app.conf.csp);
+            res.header('x-content-security-policy', app.conf.csp);
+            res.header('x-webkit-csp', app.conf.csp);
+        }
+        sUtil.initAndLogRequest(req, app);
+        next();
+    });
+
     // disable the X-Powered-By header
     app.set('x-powered-by', false);
     // disable the ETag header - users should provide them!
@@ -106,25 +125,6 @@
     app.use(bodyParser.json());
     // use the application/x-www-form-urlencoded parser
     app.use(bodyParser.urlencoded({extended: true}));
-
-    // set the CORS and CSP headers
-    app.all('*', function(req, res, next) {
-        if(app.conf.cors !== false) {
-            res.header('access-control-allow-origin', app.conf.cors);
-            res.header('access-control-allow-headers', 'accept, 
x-requested-with, content-type');
-            res.header('access-control-expose-headers', 'etag');
-        }
-        res.header('x-xss-protection', '1; mode=block');
-        res.header('x-content-type-options', 'nosniff');
-        res.header('x-frame-options', 'SAMEORIGIN');
-        res.header('content-security-policy', app.conf.csp);
-        res.header('x-content-security-policy', app.conf.csp);
-        res.header('x-webkit-csp', app.conf.csp);
-
-        sUtil.initAndLogRequest(req, app);
-
-        next();
-    });
 
     // serve static files from static/
     app.use('/static', express.static(__dirname + '/static'));
@@ -154,42 +154,43 @@
 function loadRoutes (app) {
 
     // get the list of files in routes/
-    return fs.readdirAsync(__dirname + '/routes')
-        .map(function (fname) {
-            return BBPromise.try(function () {
-                // ... and then load each route
-                // but only if it's a js file
-                if(!/\.js$/.test(fname)) {
-                    return undefined;
-                }
-                // import the route file
-                var route = require(__dirname + '/routes/' + fname);
-                return route(app);
-            }).then(function (route) {
-                if(route === undefined) {
-                    return undefined;
-                }
-                // check that the route exports the object we need
-                if (route.constructor !== Object || !route.path || 
!route.router || !(route.api_version || route.skip_domain)) {
-                    throw new TypeError('routes/' + fname + ' does not export 
the correct object!');
-                }
-                // wrap the route handlers with Promise.try() blocks
-                sUtil.wrapRouteHandlers(route.router);
-                // determine the path prefix
-                var prefix = '';
-                if(!route.skip_domain) {
-                    prefix = '/:domain/v' + route.api_version;
-                }
-                // all good, use that route
-                app.use(prefix + route.path, route.router);
-            });
-        }).then(function () {
-            // catch errors
-            sUtil.setErrorHandler(app);
-            // route loading is now complete, return the app object
-            return BBPromise.resolve(app);
+    return fs.readdirAsync(__dirname + '/routes').map(function(fname) {
+        return BBPromise.try(function() {
+            // ... and then load each route
+            // but only if it's a js file
+            if(!/\.js$/.test(fname)) {
+                return undefined;
+            }
+            // import the route file
+            var route = require(__dirname + '/routes/' + fname);
+            return route(app);
+        }).then(function(route) {
+            if(route === undefined) {
+                return undefined;
+            }
+            // check that the route exports the object we need
+            if(route.constructor !== Object || !route.path || !route.router || 
!(route.api_version || route.skip_domain)) {
+                throw new TypeError('routes/' + fname + ' does not export the 
correct object!');
+            }
+            // wrap the route handlers with Promise.try() blocks
+            sUtil.wrapRouteHandlers(route.router);
+            // determine the path prefix
+            var prefix = '';
+            if(!route.skip_domain) {
+                prefix = '/:domain/v' + route.api_version;
+            }
+            // all good, use that route
+            app.use(prefix + route.path, route.router);
         });
+    }).then(function () {
+        // catch errors
+        sUtil.setErrorHandler(app);
+        // route loading is now complete, return the app object
+        return BBPromise.resolve(app);
+    });
+
 }
+
 
 /**
  * Creates and start the service's web server
@@ -216,6 +217,7 @@
 
 }
 
+
 /**
  * The service's entry point. It takes over the configuration
  * options and the logger- and metrics-reporting objects from
diff --git a/config.dev.yaml b/config.dev.yaml
index 5942320..2344f6e 100644
--- a/config.dev.yaml
+++ b/config.dev.yaml
@@ -41,6 +41,8 @@
       # cors: false
       # to restrict to a particular domain, use:
       # cors: restricted.domain.org
+      # content for the CSP headers
+      # csp: false  # uncomment this line to disable sending them
       # URL of the outbound proxy to use (complete with protocol)
       # proxy: http://my.proxy.org:8080
       # the list of domains for which not to use the proxy defined above
diff --git a/doc/deployment.md b/doc/deployment.md
index bb93f7d..1c66a08 100644
--- a/doc/deployment.md
+++ b/doc/deployment.md
@@ -63,6 +63,11 @@
 Depending on the exact machine on which your service will be deployed, you may
 need to set `target` to either `ubuntu` or `debian`.
 
+If you want to specify a version of Node.JS, different from the official 
distribution
+package, set the value of the `node` stanza to the desired version, following
+[nvm](https://github.com/creationix/nvm) versions naming.
+To explicitly force official distribution package, `"system"` version can be 
used.
+
 The important thing is keeping the `dependencies` field up to date at all 
times.
 There you should list all of the extra packages that are needed in order to
 build the npm module dependencies. The `_all` field denotes packages which
@@ -72,6 +77,7 @@
 ```javascript
 "deploy": {
   "target": "ubuntu",
+  "node": "system",
   "dependencies": {
     "ubuntu": ["pkg1", "pkg2"],
     "debian": ["pkgA", "pkgB"],
@@ -117,6 +123,13 @@
 `mediawiki/services/name_in_gerrit` when checking it out in the deploy
 repository.
 
+The deploy-repo builder script assumes the name of the remote to check out in
+the deploy repository is `origin`. An alternative name can be configured by
+invoking (in the source repository):
+```
+git config deploy.remote deploy_repo_remote_name
+```
+
 ## Testing
 
 Before updating the deploy repository you need to make sure your configuration
@@ -132,8 +145,14 @@
 
 ## Update
 
-The final step is updating the deploy repository. From the source repository
-run:
+The final step is updating the deploy repository. First. make sure that your
+source repository has got the latest dependencies installed:
+
+```
+rm -rf node_modules/ && npm install
+```
+
+Update the deploy repository by running from the source repository:
 
 ```
 ./server.js build --deploy-repo
diff --git a/lib/util.js b/lib/util.js
index b9bd2a9..92c8292 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -88,6 +88,11 @@
     ret.type = err.type;
     ret.detail = err.detail;
 
+    // log the stack trace only for 500 errors
+    if(Number.parseInt(ret.status) !== 500) {
+        ret.stack = undefined;
+    }
+
     return ret;
 
 }
diff --git a/package.json b/package.json
index dd34436..5c3fdea 100644
--- a/package.json
+++ b/package.json
@@ -36,29 +36,29 @@
   },
   "homepage": "https://github.com/wikimedia/mathoid";,
   "dependencies": {
-    "bluebird": "^3.0.6",
-    "body-parser": "^1.14.1",
+    "bluebird": "^3.1.1",
+    "body-parser": "^1.14.2",
     "bunyan": "^1.5.1",
     "cassandra-uuid": "^0.0.2",
-    "compression": "^1.6.0",
-    "domino": "^1.0.19",
+    "compression": "^1.6.1",
+    "domino": "^1.0.21",
     "express": "^4.13.3",
-    "js-yaml": "^3.4.3",
-    "preq": "^0.4.4",
-    "service-runner": "^0.3.5",
+    "js-yaml": "^3.5.2",
+    "preq": "^0.4.8",
+    "service-runner": "^0.3.8",
     "mathoid-mathjax-node": "0.4.4",
     "texvcinfo": "^0.3.3"
   },
   "devDependencies": {
     "extend": "^3.0.0",
-    "istanbul": "^0.4.1",
-    "mocha": "^2.3.3",
-    "mocha-jshint": "^2.2.3",
+    "istanbul": "^0.4.2",
+    "mocha": "^2.3.4",
+    "mocha-jshint": "^2.2.6",
     "mocha-lcov-reporter": "^1.0.0",
-    "swagger-router": "^0.3.3",
-    "commander": "~2.9.0",
-    "xmldom": "~0.1.19",
-    "dom-compare":"~0.2.1"
+    "swagger-router": "^0.3.4",
+    "commander": "^2.9.0",
+    "xmldom": "^0.1.21",
+    "dom-compare":"^0.2.1"
   },
   "deploy": {
     "node": "4.2",
diff --git a/test/features/app/app.js b/test/features/app/app.js
index 5f9927d..47f9134 100644
--- a/test/features/app/app.js
+++ b/test/features/app/app.js
@@ -26,6 +26,9 @@
     });
 
     it('should set CORS headers', function() {
+        if(server.config.service.conf.cors === false) {
+            return true;
+        }
         return preq.get({
             uri: server.config.uri + 'robots.txt'
         }).then(function(res) {
@@ -37,6 +40,9 @@
     });
 
     it('should set CSP headers', function() {
+        if(server.config.service.conf.csp === false) {
+            return true;
+        }
         return preq.get({
             uri: server.config.uri + 'robots.txt'
         }).then(function(res) {
diff --git a/test/utils/server.js b/test/utils/server.js
index 51888e3..06c6063 100644
--- a/test/utils/server.js
+++ b/test/utils/server.js
@@ -23,6 +23,7 @@
 var myServiceIdx = config.conf.services.length - 1;
 var myService = config.conf.services[myServiceIdx];
 config.uri = 'http://localhost:' + myService.conf.port + '/';
+config.service = myService;
 // no forking, run just one process when testing
 config.conf.num_workers = 0;
 // have a separate, in-memory logger only

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I252f2a71929b6919d4f577d365517cae327a8c60
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/mathoid
Gerrit-Branch: master
Gerrit-Owner: Mobrovac <[email protected]>
Gerrit-Reviewer: JanZerebecki <[email protected]>
Gerrit-Reviewer: Mobrovac <[email protected]>
Gerrit-Reviewer: Physikerwelt <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to