Merge pull request #5 from ginkgobioworks/fix/streams-bug

Fix minor bugs in Bunyan config streams handling

lib/arguments.js

@@ -36,5 +36,13 @@ module.exports = {
   debug: {
     alias: 'd',
     describe: 'Force run the server in development mode'
-  }
+  },
+  ip: {
+    alias: 'i',
+    describe: 'IP server runs on [default: 0.0.0.0]'
+  },
+  rooturlpath: {
+    alias: 'r',
+    describe: 'Root URL path server is deployed on; will be removed from URL when resolving to files [default: /]'
+  },
 };

lib/config.js

@@ -2,57 +2,48 @@
 
 var nconf = require('nconf');
 
-//Load arguments as highest priority
+// Load arguments as highest priority
 nconf.argv(require('./arguments'));
 
-//Overrides
+// Overrides
 var overrides = {};
 
 if (nconf.get('prod')) {
   overrides.NODE_ENV = 'production';
-}
+} else if (nconf.get('debug')) {
-else if (nconf.get('debug')) {
   overrides.NODE_ENV = 'development';
 }
 
-//Load overrides as second priority
+// Load overrides as second priority
 nconf.overrides(overrides);
 
-
+// Load enviroment variables as third priority
-//Load enviroment variables as third priority
 nconf.env();
 
-
+// Load the config if it exists.
-//Load the config if it exists.
 nconf.file(nconf.get('config') || './config.json');
 
-
+// Default variables
-//Default variables
 nconf.defaults({
   name: nconf.get('name') || 'spserver',
   NODE_ENV: 'development',
+  ip: '0.0.0.0',
   production: {
     port: 80,
     bunyan: {
       name: nconf.get('name') || 'spserver',
-      streams: [{
+      stream: 'process.stdout',
-          stream: 'process.stdout',
+      level: 'info',
-          level: 'info'
-        }
-      ]
     },
   },
   development: {
     port: 3001,
     bunyan: {
       name: nconf.get('name') || 'spserver',
-      streams: [{
+      stream: 'process.stdout',
-          stream: 'process.stdout',
+      level: 'debug',
-          level: 'debug'
-        }
-      ]
     },
-  }
+  },
 });
 
 module.exports = nconf;

lib/logger.js

@@ -13,9 +13,22 @@ var output;
 if (config.get('bunyan') || config.get(env + ':use_bunyan')) {
   var settings = _.cloneDeep(config.get(env + ':bunyan'));
 
-  // Stream can be specified either in settings.streams[ix] or globally in settings.stream
+  // Stream can be specified either in settings.streams[ix] or globally in settings.stream, but not
+  // both. Since the defaults specify stetings.stream, if the user specifies anything of vaulue
+  // in settings.streams, we should delete the global defaults, because bunyan gets angry if there
+  // are multiple keys
+  if (_.has(settings, 'streams')) {
+    if (settings.streams) {
+      delete settings.stream;
+      delete settings.level;
+    } else {
+      delete settings.streams;
+    }
+  }
+
   _([settings.streams, settings])
     .flatten()
+    .compact()
     .forEach(function (settingObj) {
       if (settingObj.stream === 'process.stdout') {
         settingObj.stream = process.stdout;

lib/spserver.js

@@ -2,17 +2,24 @@
 
 var fs = require('fs');
 var http = require('http');
+var path = require('path');
+var url = require('url');
+
 var _ = require('lodash');
 var nStatic = require('node-static');
-var path = require('path');
-
 
 var config = require('./config');
 var logger = require('./logger');
 
-var env = config.get('NODE_ENV');
 
-var spserver = function (settings) {
+// The different config sources sometimes manipulate different setting names.
+// E.g. command line flags maniuplate root settings, but config files can
+// manipulate settings at the prod/debug level. Resolve all of these into a
+// final object of settings.
+function _resolveFinalSettings(settings) {
+  var finalSettings = {};
+  var env = config.get('NODE_ENV');
+
   if (!settings) {
     settings = config.get();
   }
@@ -20,19 +27,80 @@ var spserver = function (settings) {
     settings[env] = {};
   }
 
-  var fileServer = new nStatic.Server(path.resolve(settings.serve || settings[env].server));
+  // For 'name', 'file', 'serve', 'ip', and 'port', default to the global setting rather than an
+  // individual environment's setting, because it might have been set via command-line flags
+  _(['name', 'file', 'serve', 'ip', 'port']).forEach(function (field) {
+    finalSettings[field] = settings[field] || settings[env][field];
+  });
 
-  var base = generateBase(path.resolve(settings.file || settings[env].file), settings);
+  finalSettings.rooturlpath = config.get('rooturlpath') || config.get('ROOT_URL_PATH') || '/';
+
+  // For 'staticOptions', there are no command-line flags, so individual configuration options
+  // override global defaults where set
+  finalSettings.staticOptions = _.defaultsDeep(settings.staticOptions, settings[env].staticOptions);
+
+  // Make a template function so we can just pass that in downstream
+  finalSettings.template = (settings.template || settings[env].template) ?
+    function (contents) {
+      // Note: template is run with _original_, non-resolved settings
+      return _.template(contents)(settings);
+    } : null;
+
+  return finalSettings;
+}
+
+function generateBase(file, finalSettings) {
+  if (!file) {
+    return null;
+  }
+
+  if (_.endsWith(file, 'js')) {
+    return require(file);
+  }
+
+  var contents = fs.readFileSync(file);
+  if (finalSettings.template) {
+    contents = finalSettings.template(contents);
+  }
+
+  return function(req, res) {
+    res.writeHead(200, {'Content-Type': 'text/html'});
+    res.end(contents);
+  };
+}
+
+function _rerouteRootUrl(reqUrl, rootUrl) {
+  var parsedUrl = url.parse(reqUrl);
+
+  parsedUrl.pathname = path.normalize(
+    parsedUrl.pathname.replace(rootUrl, '/') || '/'
+  );
+
+  return url.format(parsedUrl);
+}
+
+var spserver = function (settings) {
+  var finalSettings = _resolveFinalSettings(settings);
+
+  var fileServer = new nStatic.Server(
+    path.resolve(finalSettings.serve),
+    finalSettings.staticOptions
+  );
+
+  var base = generateBase(path.resolve(finalSettings.file), finalSettings);
 
   var server = http.createServer(function (req, res) {
-    logger.debug('[REQ] GET:', req.url);
+    logger.debug('[REQ]', req.method + ':', req.url);
     var startTime = new Date().getTime();
 
     var done = function () {
       var requestTime = new Date().getTime() - startTime;
-      logger.debug('[RES] GET:', req.url, '(' + res.statusCode + ') took', requestTime, 'ms');
+      logger.debug('[RES]', req.method + ':', req.url,
+        '(' + res.statusCode + ')', 'took', requestTime, 'ms');
     };
 
+    req.url = _rerouteRootUrl(req.url, finalSettings.rooturlpath);
+
     res.addListener('finish', done);
     res.addListener('close', done);
 
@@ -45,44 +113,25 @@ var spserver = function (settings) {
             logger.error(err);
 
             res.writeHead(err.status, err.headers);
-            res.end();
+            res.end(err.message);
           }
         }
       });
     }).resume();
   });
 
-  server.listen(settings.port || settings[env].port);
+  server.listen(finalSettings.port, finalSettings.ip);
 
-  logger.info('Static server',
+  logger.info(
-              settings.name,
+    'Started single-page server: ' + finalSettings.name +
-              'is listening on port',
+    ', base file: ' + finalSettings.file +
-              settings.port || settings[env].port,
+    ', static folder: ' + finalSettings.serve +
-              'with public folder',
+    ', port: ' + finalSettings.port
-              settings.serve || settings[env].serve);
+  );
+
+  return server;
 };
 
 spserver.generateBase = generateBase;
 
-function generateBase(file, settings) {
-  if (!file) {
-    return null;
-  }
-
-  if (_.endsWith(file, 'js')) {
-    return require(file);
-  }
-
-  var contents = fs.readFileSync(file);
-
-  if (settings.template || settings[env] && settings[env].template) {
-    contents = _.template(contents)(settings);
-  }
-
-  return function(req, res) {
-    res.writeHead(200, {'Content-Type': 'text/html'});
-    res.end(contents);
-  };
-}
-
 module.exports = spserver;

package.json

@@ -22,8 +22,8 @@
   "homepage": "https://github.com/TheThing/spserver",
   "dependencies": {
     "bunyan": "^1.3.3",
-    "lodash": "^3.0.1",
+    "lodash": "^4.17.2",
-    "nconf": "^0.7.1",
+    "nconf": "^0.8.4",
     "node-static": "^0.7.6"
   },
   "bin": "./bin.js",

test/base.test.js

@@ -4,19 +4,19 @@ var fs = require('fs');
 var assert = require('assert');
 var sinon = require('sinon');
 
-describe('spserver', function() {
+describe('spserver', function () {
 
   var spserver = require('../lib/spserver');
 
-  describe('#generateBase()', function() {
+  describe('#generateBase()', function () {
-    it('should return null when file is empty', function() {
+    it('should return null when file is empty', function () {
       assert.strictEqual(null, spserver.generateBase());
       assert.strictEqual(null, spserver.generateBase(null, {}));
       assert.strictEqual(null, spserver.generateBase(''));
       assert.strictEqual(null, spserver.generateBase('', {}));
     });
 
-    it('should read file contents if string', function() {
+    it('should read file contents if string', function () {
       var stub = sinon.stub(fs, 'readFileSync').returns('bla');
 
       spserver.generateBase('asdf', {});
@@ -24,7 +24,7 @@ describe('spserver', function() {
       stub.restore();
     });
 
-    it('should return function if file is javascript', function() {
+    it('should return function if file is javascript', function () {
       var path = require('path');
       var nothing = require('./nothing');
       var test = spserver.generateBase(path.resolve('test/nothing.js'), {});