Using Codepainter to Format your Node.js App

I previously posted about using jshint to catch style errors in your code. To be sure, it’s way, way, way better to have a style formatter automatically apply your chosen style right within your editor–or at least flag your errors as soon as possible after you make them. But, neither of these will help you if you have a bunch of code that you need to get into shape; then you need a tool that will actually transform your code in abatch. Codepainter will apply selected transforms to your code as a grunt task–giving you improved code as a result. There are a few caveats, but it worked for me. First, install codepainter and grunt-codepainter using npm. Codepainter gets configured in grunt like this:


codepainter: {
static: {
options: {
editorConfig: false,
style: {
indent_style: 'tab',
trim_trailing_whitespace : true,
indent_size : 1,
max_line_length : 100,
quote_type : 'single',
curly_bracket_next_line : false,
spaces_around_operators : true,
space_after_control_statements : true,
space_after_anonymous_functions : true,
spaces_in_brackets : false
}
},
files: [{
expand: true, // Enable dynamic expansion
cwd: 'app/', // Src matches are relative to this path
src: ['+(controllers|services)/**/*.js'], // Actual patterns to match
dest: 'styled/' // Destination path prefix
}]
},
},

Add a line to register the task:

grunt.registerTask('styleme', 'codepainter');

That’s it. When your run grunt styleme, codepainter will apply the specified style transforms to the files that it finds using the cwd: and src: entries, and puts the transformed files in the directory you specify in dest:. It preserves the directory structure in the output, so I ended up with styled/controllers and styled/services, including subdirectories. Copy these directories back into the directory you specified in cwd:, and you now have the styled files in your project. You should, of course, back up those directories beforehand, copy the files over, and then run all of your tests afterwards. You can use grunt-contrib-copy and set up a chain of tasks to format, copy, and test.

But use extreme caution, here. This is not something I can recommend that you do regularly–if you do–please set it up so that it’s automated and goof-proof! The real holy grail for me remains something that will correct my style as I write, and flag errors that it can’t correct. But codepainter will save you from having to make thousands of style corrections by hand, if you ever face such a situation!

Advertisements
Posted in Software | Tagged | Leave a comment

Complexity Analysis for Node.js Apps

In previous posts, I introduced style checking (linting) using jshint, integration testing using mocha and should, code test coverage using coverage, and vulnerability identification using retire. If you’re starting a new project, I strongly encourage you to integrate these packages into your routine–it will save you plenty in the long run.

I now want to turn to managing software complexity. Defect rates increase superlinearly to code complexity, so managing software complexity is part of a broader strategy of risk reduction. Software complexity is not as cut & dried as style checking and unit testing, and refactoring requires effort and entails its own risks. Automated complexity measures primary benefit is that it help focus your attention on high-risk sections of your code. A thorough treatment of the topic is beyond the scope of a blog post, but you can get started quite easily.

I use two node packages: ‘complexity-report’ and ‘grunt-complexity’. Install them using npm, and then set up your gruntfile:


complexity: {
generic: {
src: ['app/**/*.js', 'public/modules/*.js'],
exclude: ['app/yyy/**', 'app/tests/**', 'app/services/xxx/**' ],
options: {
breakOnErrors: true,
//jsLintXML: 'report.xml', // create XML JSLint-like report
//checkstyleXML: 'checkstyle.xml', // create checkstyle report
//pmdXML: 'pmd.xml', // create pmd report
errorsOnly: false, // show only maintainability errors
cyclomatic: [10, 20, 50], // or optionally a single value, like 3
halstead: [10, 20, 50], // or optionally a single value, like 8
maintainability: 75,
hideComplexFunctions: true, // only display maintainability. Set to false to get more detailed output.
broadcast: false // broadcast data over event-bus
}
}
}

...and...

grunt.registerTask('complex', 'complexity');

Note: you cannot use grunt.registerTask(‘complexity’, ‘complexity’). It’s like crossing the beams, if you remember your Ghostbusters analogies.

If you’re just starting your new application–great! If you’re following TDD/BDD, then complexity is just another tool to help you make choices as you refactor–you’re always trying to keep the board green! (But see my caveats at the end). Using the settings above (particularly hideComplexFunctions: true), when you run grunt complex, you’ll get a nice summary of the maintainability metric for each module, with the output ordered from the lowest score to the highest. Maintainability was introduced by Paul Oman and Jack Hagemeister in 1991, and combines 3 different metrics to create an overall availability score. Don’t worry about the details for now. You can control the coloring of the output by changing the value of maintainability: 75 to something else. We’ll discuss this in a bit. Anyway, this is the output format I recommend you use–as long as everything is green, you’re good to go. In my opinion, BDD/TDD helps to prevent a lot of the issues that retrospective complexity analysis was designed to address. I use complexity-report as another safety check during development & deployment, and to help triage code for manual review.

If you’re dealing with existing code, then things are a bit more…err…complex. The first time you run grunt complex, there’s a good chance that you see a lot of red and yellow bars. Don’t panic! To understand where the trouble spots are, we have to dig a little deeper. Set hideComplexFunctions: false, and rerun grunt complex. It still provides the maintainability graphic for each module, but nested underneath each entry are the complexity metrics for every function found in that module. I don’t recommend that you attribute any prescriptive power to these metrics. Clean Code: A Handbook of Agile Software Craftsmanship, by Robert C. Martin, is a great resource for improving the maintainability of your code. The examples are in java, but the principles are universal.

If you want a better understanding of code metrics can be found here and here.

Do not try to reduce complexity purely for the sake of lowering a metric below a specified number. Adjust the thresholds based on your experience and what you (and your peers) see in the code. If you’re OK with a function, then leave it alone. Phil Booth, the author of complexity-report, has accepted my suggestion to allow module and function-level overrides of the global threshold values for the individual metrics, similar to what jshint allows. This is an important feature, because you can then put complexity-report into your standard development & deployment practices, setting thresholds to sensible values in various places. Currently, you have to do complexity reporting out-of-band, or set ridiculously high thresholds that render the capability useless for deployment.

Posted in Software | Tagged | Leave a comment

Vulnerability Detection for Node.js Apps Using Retire

Eliminating from your application all code–include node modules–with known vulnerabilities is an essential part of secure coding. Retire is a node.js package that looks for reported vulnerabilities in your node modules and/or javascript code. It’s dirt simple to use–so there’s no excuse for not using it.

Just install retire using NPM. If you use grunt, install grunt-retire as well.

The gruntfile config entry looks like this

retire: {
js: ['app/**/*.js'], /** Which js-files to scan. **/
node: ['node_modules'], /** Which node directories to scan (containing package.json). **/
options: {
verbose: true,
packageOnly: false,
jsRepository: 'https://raw.github.com/bekk/retire.js/master/repository/jsrepository.json',
nodeRepository: 'https://raw.github.com/bekk/retire.js/master/repository/npmrepository.json',
}
},

You may have to change the js and node directories.

To enable the task, add this in grunt:

grunt.registerTask('retireCheck', ['retire']);

Note that grunt.registerTask(‘retire’, [‘retire’]); will NOT work!

Now run grunt retireCheck. Make this part of your build process to reduce the potential for vulnerable code to ruin your day!

Posted in Software | Tagged , | 1 Comment

Style Checking (linting) Node.js Apps

Style checking (aka linting) is a vital part of any application development process. Sloppy coding practices increase the chances of coding errors and other problems later on, and style checking can help flag many coding problems to keep your code clean. Why is it so important to keep your code clean and uniform? Simple: While you are building a system, you create a mental model of that system-uniform style helps reduce the cognitive effort required to translate the code into your mental model. It’s less taxing on your brain! Consequently, you have thought power to apply to what you’re trying to build, and you’re more likely to build something that works properly.

For node.js, I use the jshint tool. For grunt integration I use grunt-contrib-jshint. The gruntfile entry looks like this:


jshint: {
all: {
src: ['app/**/*.js', '!app/tests/**'],
options: {
//reporter: 'checkstyle',
jshintrc: true
}
}
}

Look at the src: element. The first entry tells jshint to look for all js files in all subdirectories of app, while the second entry tells jshint to skip my tests directory (and subdirectories).

Now look at jshintrc: true — that tells jshint that there is a file .jshintrc in the app’s root directory that contains configuration parameters. Here’s a jshintrc file:

{
"quotmark": "single", // Define quotes to string values.
"bitwise": true, // Prohibit bitwise operators (&, |, ^, etc.).
"noempty": true, // Prohibit use of empty blocks.
"curly": true, // Require {} for every new block or scope.
"eqeqeq": true, // Require triple equals i.e. `===`.
"smarttabs": true, // Suppresses warnings about mixed tabs and spaces
"trailing": true, // Prohibit trailing whitespaces.
"unused": true, // Warn unused variables.
"immed": true, // Require immediate invocations to be wrapped in parens e.g. `( function(){}() );`
"latedef": "nofunc", // Prohibit variable use before definition.
"newcap": true, // Require capitalization of all constructor functions e.g. `new F()`.
"noarg": true, // Prohibit use of `arguments.caller` and `arguments.callee`.
"regexp": true, // Prohibit `.` and `[^...]` in regular expressions.
"undef": true, // Require all non-global variables be declared before they are used.
"camelcase": false, // Permit only camelcase for `var` and `object indexes`.
"globals": { // Globals variables.
"jasmine": true,
"angular": true,
"ApplicationConfiguration": true
},
"predef": [ // Extra globals.
"define",
"require",
"exports",
"module",
"describe",
"before",
"beforeEach",
"after",
"afterEach",
"it",
"inject",
"expect"
],
"node": true, // Enable globals available when code is running inside of the NodeJS runtime environment.
"strict": true, // Require `use strict` pragma in every file.
"browser": true, // Standard browser globals e.g. `window`, `document`.
"esnext": true, // Allow ES.next specific features such as `const` and `let`.
"devel": true, // Allow development statements e.g. `console.log();`.
"indent": 4 // Specify indentation spacing
}

If you didn’t use jshint from the very beginning, you’re in for a big surprise when you first run jshint. Don’t panic!
1) In the .jshintrc file, set to false all of the entries after quotmark up to globals.
2) Run lint and fix the problems that show up.
3) Set one of the entries to true.
4) Repeat steps 2 & 3, working your way through until you’ve corrected all of your code.

camelcase comes last, because there are side-effects–you have to change multiple lines at once and make sure that you’re not causing other issues–like renaming a variable to a variable that’s already defined. Another trouble spot is if you’re preparing a json structure and sending that to the client–you have to change the client code (eg: angular templates, etc.) that’s processing that json at the same time. If you have unit tests defined, this is a good time to use them liberally.

The reason why I suggest following the above and fixing one type of problem throughout your codebase and then moving on to the next problem is that it lets you focus on making one type of change, then a different type of change, etc. Your mind can focus on just the one thing. If you want to correct all issues in one module before moving on to the next, change the src: entry so that only one module is linted at a time. Otherwise, the output is difficult to read, and you could end up making a change in the wrong module.

Global settings will only get you so far. There will come a point where you want to permanently ignore a specific warning for a specific line of code. Let’s take “strict”: true, for example. I have a few modules where I assign things to ‘this’:


function myFunction(task) {
this.key = task.key;
this.userId = task.userId;
this.name = task.name;
...
}

Assigning to ‘this’ will generate a strict violation, but I am OK with that here! You can suppress a warning by doing the following:
1) In gruntfile.js, uncomment the reporter: ‘checkstyle’ entry.
2) Run your lint task.
3) The output is difficult to read, but each entry will include a warning code at the end, eg: W040.
4) In the source code in which you want to suppress this specific warning, add: /* jshint -W040 */
5) To re-enable this warning elsewhere in that same module, add: /* jshint +W040 */. Try to limit the scope of these suppressions.
6) No other module is affected by these hints.

Posted in Software | Tagged , | Leave a comment

Node.js Tests using Mocha

In this post, I want to talk a little more about basic Mocha tests. Here are two tests in my mail.service.test file:


var mail = require('app/services/mail.service');
var config = require('../../config/config');
var should = require('should');

describe('Send Mail', function() {

it('should send a valid email via SMTP and return true', function(done) {
config.useMailer = 'SMTP';
config.requireModules.nodemailer.sendMailError = false;
mail.sendMail(config.testMailer.FromAddress, config.testMailer.ToAddresses,config.testMailer.subject, config.testMailer.body, function(res) {
res.should.be.true;
});
done();
});

it('should fail via SMTP when the mail service encounters an error', function(done) {
config.useMailer = 'SMTP';
config.requireModules.nodemailer.sendMailError = true;
mail.sendMail(config.testMailer.FromAddress, config.testMailer.ToAddresses,config.testMailer.subject, config.testMailer.body, function(res) {
res.should.be.false;
});
done();
});
});

Very roughly, the ‘describe’ is sort of equivalent to a Gherkin ‘feature’, while ‘it’ is sort of equivalent to a Gherkin scenario, even though cucumber and Mocha work in entirely different ways.

Let’s look at the first test in more detail. My app can use multiple mailing methods. config.useMailer = ‘SMTP’ just chooses the SMTP method. I’ll show you the simplified module in a bit. config.requireModules.nodemailer.sendMailError = false is only used by my mock mailer. Note in the second test it’s set to true, which merely forces the mock mailer to return an error no matter what it’s passed. The mail.sendMail(…) is the actual attempt to send an email. That’s it for the test.

Here’s a simplified version of my mail service:


var config = require('../../config/config');
var log = require('npmlog');

var nodemailer = require(config.requireModules.nodemailer.version);

exports.sendMail = function(mail_from, mail_to, mail_subject, mail_body) {
var status = false;
if (config.useMailer === 'SMTP') {
var smtpTransport = nodemailer.createTransport(config.mailer.options);
var mailOptions = {
to: mail_to,
from: mail_from,
subject: mail_subject,
html: mail_body
};
smtpTransport.sendMail(mailOptions, function(error, info) {
if (error) {
log.error('error sending email via SMTP to:' + mail_to + ' about ' + mail_subject + '\nError: ' + error + '\n\n\n');
status = false;
} else {
log.info('email sent via SMTP to:' + mail_to + ' about ' + mail_subject + ' info: ' + info.response + '\n\n\n');
status = true;
};
});
}
return status;
};

Look at the top: var nodemailer = require(config.requireModules.nodemailer.version);

This pulls the name of the module to require out of the config file. For testing, I put the mock module name in the config file. For testing, I have a mock nodemailer module and set the path to it in the config file. I posted about this earlier.

The mock nodemailer looks like this:


var config = require('config/config');

module.exports.createTransport = function(transporter) {
return new Nodemailer(transporter);
};

function Nodemailer(transporter) {
this.mock = 'Mock-Nodemailer!';
this.transporter = transporter;
};

Nodemailer.prototype.sendMail = function(data, callback) {
callback = callback || function() {};
var info = {};
var error;
if (config.requireModules.nodemailer.sendMailError) {
info.response = 'ERROR';
error = true;
} else {
info.response = 'OK';
error = null;
}
return callback(error, info);
};

So, these two tests ensure that both of the condition branches (ie: the error check in the sendMail callback) get tested and the correct response code is returned.

Posted in Software, Uncategorized | Tagged , , , | Leave a comment

Creating Tests for a Node.js App After It’s Built

For some reason, you have a node.js app that’s built, but it has no tests or documentation. Maybe you inherited it. Maybe you were feeling exceptionally manly and didn’t think you needed tests. Maybe you’re now having second thoughts or whatever. It’s thankfully not that difficult–and certainly worth the effort–to add tests after the fact, even though I think behavior-driven design is a better option.

For a node app that I inherited, I started by building lower-level tests–I hesitate to call them unit tests–focusing on the parts of the code that I felt would be the most difficult for someone else to understand–or the most difficult for me to remember if I left it for some time. Starting this way also helped me to get my head around the code. Along the way, I discovered that certain parts of the system weren’t testable, so I refactored them. I ended up with over 120 tests, better code, and peace of mind.

I started with mocha and should, and then added istanbul for code coverage–all run from within grunt. I chose these primarily because I could get them to work and found enough examples to feel comfortable using them. Mocha and should tests seemed very natural to write–I’m still not sure what to think about cucumber using Gherkin-syntax for these lower-level tests. Istanbul was extremely useful in figuring out what tests I still needed to write–but note that its coverage metrics only pertain to the modules for which tests are written and not to the entire codebase.

These tests are pretty fast, taking about 10 seconds to run, including app initialization. This isn’t nearly fast enough for true Test-driven development, but I’ve somehow inherited 2 other node apps for which I need to build tests–this will have to do for now.

I did encounter a few issues: The biggest so far is that a few tests that run fine in mocha explode in istanbul. I tried a few things, but eventually gave in to my inner slacker and moved these tests-of-death into tests/mocha-only and configured istanbul to only run tests in the tests directory.

Here are the packages:
cucumber
grunt-cucumber
grunt-mocha-istanbul
grunt-mocha-test
istanbul
should
webdriverio

These are configured in gruntfile.js (partial file, below):

mochaTest: {
src: watchFiles.mochaTests,
options: {
reporter: 'spec',
require: 'server.js'
}
},
mocha_istanbul: {
coverage: {
src: 'app/', // a folder works nicely
options: {
mask: 'tests/*test.js'
}
}
},
cucumberjs: {
src: 'features',
options: {
format: 'pretty', //pretty : prints the feature as is,
//progress: prints one character per scenario,
//json : prints the feature as JSON,
//summary : prints a summary only, after all scenarios were executed
steps: 'features/steps'
}
},
istanbul_check_coverage: {
default: {
options: {
coverageFolder: 'coverage*', // will check both coverage folders and merge the coverage results
check: {
lines: 90,
statements: 90,
branches: 75
}
}
}

...

// Test task.
grunt.registerTask('test', ['env:test', 'mochaTest']);

grunt.registerTask('accept', ['env:test', 'cucumberjs']);

//Code coverage
grunt.registerTask('coverage', ['env:test','mocha_istanbul:coverage']);

grunt.registerTask('check_coverage', ['env:test', 'istanbul_check_coverage']);

All of these tests are written against my service modules–essentially these service modules support my mean.js controllers. Once I had good enough low-level test coverage, and wanting an excuse to try building high-level acceptance tests using cucumber, I moved on.

I’m using cucumber.js, selenium server, and webdriverio for my acceptance tests. Again, the primary factor in choosing these particular pieces was that I could get them installed and working, and found enough examples and documentation to feel comfortable going forward. Now, the BDD folks will tell you that the primary benefit of BDD is NOT that the specifications can be executed, but the conversations among team members that arise from following the method. This is a good thing, because at the moment I only have 7 tests working out of an initial 14, and the tests take practically forever to run. I clearly have something wrong, somewhere.

I’ll post some details about the mocha tests, then I’ll turn to the cucumber tests…

Posted in Software | Tagged , | Leave a comment

Node.js Secure Coding – Disabling Authentication

This is a snippet of a function that performs user authentication (login).

What’s wrong here?


module.exports.authenticate = function(req, res, next) {
if (env !== 'production') {
return next();
}
...

Give up?
You want to default to a more secure posture, not less. If you need to disable authentication, you must, Must, MUST be explicit as to when, and you must, Must, MUST default to more secure. The above should look something like:


module.exports.authenticate = function(req, res, next) {
if (env === 'development' || env === 'testing' ) {
return next();
}
...

This problem is not restricted to node, obviously. It’s a basic premise of secure coding.

Posted in Software | Tagged , | Leave a comment