From 923d1fc7232ae3414e0fe51227280d377db3de2b Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Wed, 14 Dec 2016 18:04:28 -0500 Subject: [PATCH 1/9] Respond with the correct error code and headers --- lib/logger.js | 2 +- lib/spserver.js | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/logger.js b/lib/logger.js index 8f53dc1..942992c 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -22,7 +22,7 @@ if (config.get('bunyan') || config.get(env + ':use_bunyan')) { output = bunyan.createLogger(settings); } else { output = console; - console.debug = console.log.bind(console); + output.debug = console.log.bind(console); } module.exports = output; diff --git a/lib/spserver.js b/lib/spserver.js index 0f58b63..2e7fd7f 100644 --- a/lib/spserver.js +++ b/lib/spserver.js @@ -12,7 +12,7 @@ var logger = require('./logger'); var env = config.get('NODE_ENV'); -var spserver = function(settings) { +var spserver = function (settings) { if (!settings) { settings = config.get(); } @@ -28,7 +28,7 @@ var spserver = function(settings) { logger.debug('[REQ] GET:', req.url); var startTime = new Date().getTime(); - var done = function() { + var done = function () { var requestTime = new Date().getTime() - startTime; logger.debug('[RES] GET:', req.url, '(' + res.statusCode + ') took', requestTime, 'ms'); }; @@ -36,16 +36,18 @@ var spserver = function(settings) { res.addListener('finish', done); res.addListener('close', done); - //return base(req, res); req.addListener('end', function () { - fileServer.serve(req, res, function(e) { - if (!e) return; - if (e && e.status === 404 && base) { - return base(req, res); + fileServer.serve(req, res, function (err) { + if (err) { + if (err.status === 404 && base) { + return base(req, res); + } else { + logger.error(err); + + res.writeHead(err.status, err.headers); + res.end(); + } } - logger.error(e); - res.writeHead(404); - res.end(); }); }).resume(); }); From 026e1f12892396cdf5dd0e9cc51319d7d99a410e Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 16:01:22 -0500 Subject: [PATCH 2/9] Make bin.js executable --- bin.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 bin.js diff --git a/bin.js b/bin.js old mode 100644 new mode 100755 From f4dbcb9b1204a19cd158987b80c648f71b3ca440 Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 16:58:35 -0500 Subject: [PATCH 3/9] Upgrade nconf to fix bug in config config.get().name was undefined, even though config.get('name') worked fine --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bde03d6..c4c6d70 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "dependencies": { "bunyan": "^1.3.3", "lodash": "^3.0.1", - "nconf": "^0.7.1", + "nconf": "^0.8.4", "node-static": "^0.7.6" }, "bin": "./bin.js", From 9acb178280fa63958b5f924485833842e9f3f5a3 Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 17:43:52 -0500 Subject: [PATCH 4/9] Fix minor spacing issues --- lib/config.js | 18 +++++++----------- lib/spserver.js | 4 ++-- test/base.test.js | 10 +++++----- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/lib/config.js b/lib/config.js index 9ea9cf0..4aa01e9 100644 --- a/lib/config.js +++ b/lib/config.js @@ -2,32 +2,28 @@ 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', diff --git a/lib/spserver.js b/lib/spserver.js index 2e7fd7f..b7cb21c 100644 --- a/lib/spserver.js +++ b/lib/spserver.js @@ -2,10 +2,10 @@ var fs = require('fs'); var http = require('http'); -var _ = require('lodash'); -var nStatic = require('node-static'); var path = require('path'); +var _ = require('lodash'); +var nStatic = require('node-static'); var config = require('./config'); var logger = require('./logger'); diff --git a/test/base.test.js b/test/base.test.js index 93cd57c..f4f7ad2 100644 --- a/test/base.test.js +++ b/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() { - it('should return null when file is empty', function() { + describe('#generateBase()', 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'), {}); From 051c5b4433505fa1235f5c61746f4dba15d1d6fb Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 17:48:24 -0500 Subject: [PATCH 5/9] Fix stream parsing bug In lodash 3.10, .forEach does not get called in a chain unless .value() is passed, but in lodash 4, it does. --- lib/logger.js | 1 + package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/logger.js b/lib/logger.js index 9c7fcec..8bd3387 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -16,6 +16,7 @@ if (config.get('bunyan') || config.get(env + ':use_bunyan')) { // Stream can be specified either in settings.streams[ix] or globally in settings.stream _([settings.streams, settings]) .flatten() + .compact() .forEach(function (settingObj) { if (settingObj.stream === 'process.stdout') { settingObj.stream = process.stdout; diff --git a/package.json b/package.json index c4c6d70..0e02a63 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "homepage": "https://github.com/TheThing/spserver", "dependencies": { "bunyan": "^1.3.3", - "lodash": "^3.0.1", + "lodash": "^4.17.2", "nconf": "^0.8.4", "node-static": "^0.7.6" }, From da923ae75dcef85a789b282e8597bed9e8e8f1de Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 17:52:46 -0500 Subject: [PATCH 6/9] Fix defaults resolution when shortcut settings are passed to bunyan config --- lib/config.js | 16 +++++----------- lib/logger.js | 14 +++++++++++++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/config.js b/lib/config.js index 4aa01e9..421f19c 100644 --- a/lib/config.js +++ b/lib/config.js @@ -31,24 +31,18 @@ nconf.defaults({ port: 80, bunyan: { name: nconf.get('name') || 'spserver', - streams: [{ - stream: 'process.stdout', - level: 'info' - } - ] + stream: 'process.stdout', + level: 'info', }, }, development: { port: 3001, bunyan: { name: nconf.get('name') || 'spserver', - streams: [{ - stream: 'process.stdout', - level: 'debug' - } - ] + stream: 'process.stdout', + level: 'debug', }, - } + }, }); module.exports = nconf; diff --git a/lib/logger.js b/lib/logger.js index 8bd3387..7ae8c07 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -13,7 +13,19 @@ 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() From f420e045be827fad6345f0f190d687938f47c959 Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 17:55:59 -0500 Subject: [PATCH 7/9] Refactor setting resolution from command line and config file --- lib/spserver.js | 97 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 32 deletions(-) diff --git a/lib/spserver.js b/lib/spserver.js index b7cb21c..b9f7e55 100644 --- a/lib/spserver.js +++ b/lib/spserver.js @@ -10,9 +10,15 @@ var nStatic = require('node-static'); 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,9 +26,55 @@ var spserver = function (settings) { settings[env] = {}; } - var fileServer = new nStatic.Server(path.resolve(settings.serve || settings[env].server)); + // For 'name', 'file', 'serve', 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', 'port']).forEach(function (field) { + finalSettings[field] = settings[field] || settings[env][field]; + }); - var base = generateBase(path.resolve(settings.file || settings[env].file), settings); + // 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); + }; +} + +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); @@ -52,37 +104,18 @@ var spserver = function (settings) { }).resume(); }); - server.listen(settings.port || settings[env].port); + server.listen(finalSettings.port); - logger.info('Static server', - settings.name, - 'is listening on port', - settings.port || settings[env].port, - 'with public folder', - settings.serve || settings[env].serve); + logger.info( + 'Started single-page server: ' + finalSettings.name + + ', base file: ' + finalSettings.file + + ', static folder: ' + finalSettings.serve + + ', port: ' + finalSettings.port + ); + + 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; From 5a673439573675bdf4bfbc39b0db0acb9b97ed04 Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Thu, 15 Dec 2016 17:56:28 -0500 Subject: [PATCH 8/9] Display the request method in the log: don't assume GET --- lib/spserver.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/spserver.js b/lib/spserver.js index b9f7e55..b80d982 100644 --- a/lib/spserver.js +++ b/lib/spserver.js @@ -77,12 +77,13 @@ var spserver = function (settings) { 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'); }; res.addListener('finish', done); From 0365ce1095c65ce473346906aeacc09df0357a9d Mon Sep 17 00:00:00 2001 From: Misha Wolfson Date: Fri, 16 Dec 2016 20:18:39 -0500 Subject: [PATCH 9/9] Add IP config option --- lib/arguments.js | 6 +++++- lib/config.js | 1 + lib/spserver.js | 6 +++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/arguments.js b/lib/arguments.js index ba2e31e..95e9d52 100644 --- a/lib/arguments.js +++ b/lib/arguments.js @@ -36,5 +36,9 @@ 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]' + }, }; diff --git a/lib/config.js b/lib/config.js index 421f19c..587b108 100644 --- a/lib/config.js +++ b/lib/config.js @@ -27,6 +27,7 @@ nconf.file(nconf.get('config') || './config.json'); nconf.defaults({ name: nconf.get('name') || 'spserver', NODE_ENV: 'development', + ip: '0.0.0.0', production: { port: 80, bunyan: { diff --git a/lib/spserver.js b/lib/spserver.js index b80d982..d4523fb 100644 --- a/lib/spserver.js +++ b/lib/spserver.js @@ -26,9 +26,9 @@ function _resolveFinalSettings(settings) { settings[env] = {}; } - // For 'name', 'file', 'serve', and 'port', default to the global setting rather than an + // 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', 'port']).forEach(function (field) { + _(['name', 'file', 'serve', 'ip', 'port']).forEach(function (field) { finalSettings[field] = settings[field] || settings[env][field]; }); @@ -105,7 +105,7 @@ var spserver = function (settings) { }).resume(); }); - server.listen(finalSettings.port); + server.listen(finalSettings.port, finalSettings.ip); logger.info( 'Started single-page server: ' + finalSettings.name +