Skip to content

Commit

Permalink
config(): Add safety falback for when running in production
Browse files Browse the repository at this point in the history
Recently the data layer experienced a total failure when #11490 merged
in and crashed when it failed to initialize the new middleware handler.

The fact that a key was requested for which no value existed led to
`config()` raising an `Error` which causes the entire middleware chain
to fail to build.

Now `config()` has been modified to only raise an actual `Error` when
the `NODE_ENV=developement` and instead in all other environments to
simply log a message to the browser console and return `undefined`. This
should allow us to still catch any mistakes by the obvious message while
preventing such otherwise-small errors from cascading out-of-control.
  • Loading branch information
dmsnell committed Mar 2, 2017
1 parent 44ee49c commit 3f64db7
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 7 deletions.
45 changes: 38 additions & 7 deletions server/config/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const configPath = require( 'path' ).resolve( __dirname, '..', '..', 'config' );
const path = require( 'path' );

const configPath = path.resolve( __dirname, '..', '..', 'config' );
const data = require( './parser' )( configPath, {
env: process.env.CALYPSO_ENV || process.env.NODE_ENV || 'development',
includeSecrets: true,
Expand All @@ -7,18 +9,47 @@ const data = require( './parser' )( configPath, {
} );

/**
* Return config `key`.
* Throws an error if the requested `key` is not set in the config file.
* Returns configuration value for given key
*
* If the requested key isn't defined in the configuration
* data then this will report the failure with either an
* error or a console warning.
*
* When in the 'development' NODE_ENV it will raise an error
* to crash execution early. However, because many modules
* call this function in the module-global scope a failure
* here can not only crash that module but also entire
* application flows as well as trigger unexpected and
* unwanted behaviors. Therefore if the NODE_ENV is not
* 'development' we will return `undefined` and log a message
* to the console instead of halting the execution thread.
*
* @param {String} key The key of the config entry.
* @return {Mixed} Value of config or error if not found.
* @api public
* The config files are loaded in sequence: _shared.json, {env}.json, {env}.local.json
* @see server/config/parser.js
*
* @throws {ReferenceError} when key not defined in the config (NODE_ENV=development only)
* @param {String} key name of the property defined in the config files
* @returns {*} value of property named by the key
*/
function config( key ) {
if ( key in data ) {
return data[ key ];
}
throw new Error( 'config key `' + key + '` does not exist' );

const errorMessage = (
`Could not find config value for key '${ key }'\n` +
`Please make sure that if you need it then it has a value assigned in 'config/_shared.json'`
);

if ( 'development' === process.env.NODE_ENV ) {
throw new ReferenceError( errorMessage );
}

// display console error only in a browser
// (not in tests, for example)
if ( 'undefined' !== typeof window ) {
console.error( errorMessage );
}
}

function isEnabled( feature ) {
Expand Down
41 changes: 41 additions & 0 deletions server/config/test/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* External dependencies
*/
import { expect } from 'chai';

/**
* Internal dependencies
*/
import config from '../';

const NODE_ENV = process.env.NODE_ENV;
const fakeKey = 'where did all the errors go?';

describe( '#config', () => {
afterEach( () => process.env.NODE_ENV = NODE_ENV );

it( `should throw an error when given key doesn't exist (NODE_ENV == development)`, () => {
process.env.NODE_ENV = 'development';

expect( () => config( fakeKey ) ).to.throw( ReferenceError );
} );

it( `should not throw an error when given key doesn't exist (NODE_ENV != development)`, () => {
const envs = [
'client',
'desktop',
'horizon',
'production',
'stage',
'test',
'wpcalypso',
];

envs.forEach( env => {
process.env.NODE_ENV = env;

expect( process.env.NODE_ENV ).to.equal( env );
expect( () => config( fakeKey ) ).to.not.throw( Error );
} );
} );
} );

0 comments on commit 3f64db7

Please sign in to comment.