Skip to content

Commit

Permalink
Verify the sails.config.environment and NODE_ENV settings as soon a…
Browse files Browse the repository at this point in the history
…s they're available.

If `sails.config.environment` is "production" and NODE_ENV is undefined, set NODE_ENV to production and log warning.
If `sails.config.environment` is "production" and NODE_ENV is a string other than "production", fail to lift Sails with an error.
  • Loading branch information
sgress454 committed Nov 17, 2016
1 parent 31d050d commit abbf1f7
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 16 deletions.
42 changes: 41 additions & 1 deletion lib/app/load.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var async = require('async');
var _ = require('@sailshq/lodash');
var util = require('util');
var flaverr = require('flaverr');
var __Configuration = require('./configuration');
var __initializeHooks = require('./private/loadHooks');

Expand Down Expand Up @@ -49,8 +50,26 @@ module.exports = function(sails) {
// and options that were passed in programmatically.
config: [Configuration.load],

// Verify that the combination of Sails environment and NODE_ENV is valid
// as early as possible -- that is, as soon as we know for sure what the
// Sails environment is.
verifyEnv: ['config', function(results, cb) {
// If the userconfig hook is active, wait until it's finished to
// verify the environment, since it might be set in a config file.
if (_.isUndefined(sails.config.hooks) || (sails.config.hooks !== false && sails.config.hooks.userconfig !== false)) {
sails.on('hook:userconfig:loaded', verifyEnvironment);
}
// Otherwise verify it right now. The Sails environment will be
// whatever was set on the command line, or via the sails_environment
// env var, or defaulted to "development".
else {
verifyEnvironment();
}
return cb();
}],

// Load hooks into memory, with their middleware and routes
hooks: ['config', loadHooks],
hooks: ['verifyEnv', 'config', loadHooks],

// Load actions from disk and config overrides
controller: ['hooks', sails._controller.loadActionModules],
Expand Down Expand Up @@ -164,6 +183,27 @@ module.exports = function(sails) {
return cb();
}

function verifyEnvironment() {
// At this point, the Sails environment is set to its final value,
// whether it came from the command line or a config file. So we
// can now compare it to the NODE_ENV environment variable and
// act accordingly. This may involve changing NODE_ENV to "production",
// which we want to do as early as possible since dependencies might
// be relying on that value.

// If the Sails environment is production, but NODE_ENV is undefined,
// log a warning and change NODE_ENV to "production".
if (sails.config.environment === 'production' && process.env.NODE_ENV !== 'production' ) {
if (_.isUndefined(process.env.NODE_ENV)) {
sails.log.debug('Detected Sails environment is "production", but NODE_ENV is `undefined`.');
sails.log.debug('Automatically setting the NODE_ENV environment variable to "production".');
process.env.NODE_ENV = 'production';
} else {
throw flaverr('E_INVALID_NODE_ENV', new Error('When the Sails environment is set to "production", NODE_ENV must also be set to "production" (but it was set to "' + process.env.NODE_ENV + '" instead).'));
}
}
}

/**
* Returns function which is fired when Sails is ready to go
*
Expand Down
72 changes: 57 additions & 15 deletions test/integration/lift.lower.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,14 @@ describe('sails being lifted and lowered (e.g in a test framework)', function()

});

describe('with Sails environment set to `production`, and the Node environment not set to `production`', function() {
describe('with Sails environment set to `production`, and the Node environment is `undefined`', function() {

var sailsApp;
var originalNodeEnv;

before(function() {
originalNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'development';
delete process.env.NODE_ENV;
});

after(function(done) {
Expand All @@ -237,15 +237,15 @@ describe('sails being lifted and lowered (e.g in a test framework)', function()
}
});

it('should log a warning', function(done) {
it('should change NODE_ENV to production and log a warning', function(done) {

var warns = [];
var debugs = [];
var customLogger = {
level: 'warn',
level: 'debug',
custom: {
log: console.log.bind(console),
debug: console.log.bind(console),
warn: function(msg) {warns.push(msg);}
log: function(){},
warn: function(){},
debug: function(msg) {debugs.push(msg);}
},
colors: { warn: '' },
prefixTheme: 'abbreviated'
Expand All @@ -262,19 +262,20 @@ describe('sails being lifted and lowered (e.g in a test framework)', function()
}, function(err, _sailsApp) {

if (err) { return done(err); }

sailsApp = _sailsApp;

// Assert that NODE_ENV is unchanged.
assert.equal('development', process.env.NODE_ENV);
// Assert that NODE_ENV is changed.
assert.equal(process.env.NODE_ENV, 'production');

// Assert that sails config is unchanged.
assert.equal(sailsApp.config.environment, 'production');

var foundWarning = false;
assert (_.any(warns, function(warn) {
return warn.indexOf('Detected Sails environment of `production`, but Node environment is `development`') > -1;
}), 'Did not log a warning about NODE_ENV not being set to production!');
assert (_.any(debugs, function(debug) {
return debug.indexOf('Detected Sails environment is "production", but NODE_ENV is `undefined`.') > -1;
}), 'Did not log a warning about NODE_ENV being undefined while sails environment is `production`!');
assert (_.any(debugs, function(debug) {
return debug.indexOf('Automatically setting the NODE_ENV environment variable to "production".') > -1;
}), 'Did not log a warning about NODE_ENV being set to `production`!');

return done();

Expand All @@ -284,6 +285,47 @@ describe('sails being lifted and lowered (e.g in a test framework)', function()

});

describe('with Sails environment set to `production`, and the Node environment is `development`', function() {

var sailsApp;
var originalNodeEnv;

before(function() {
originalNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'development';
});

after(function(done) {
process.env.NODE_ENV = originalNodeEnv;
if (sailsApp) {
return sailsApp.lower(done);
}
else {
return done();
}
});

it('should fail to lift sails', function(done) {

// Load `app0` deep in the `'cenote'`
Sails().load({
environment: 'production',
log: {level: 'silent'},
globals: false,
hooks: {
grunt: false,
}
}, function(err, _sailsApp) {
if (!err) { return done(new Error('Sails should have failed to lift!')); }
assert.equal(err.code, 'E_INVALID_NODE_ENV');

return done();

});//</app0.load()>

});

});


});

0 comments on commit abbf1f7

Please sign in to comment.