Node.js: should I keep `assert()`s in production code?

Question

A methodological question:

I'm implementing an API interface to some services, using node.js, mongodb and express.js.

On many (almost all) sites I see code like this:

method(function(err, data) {
  assert.equal(null, err);
});

The question is: should I keep assert statements in my code at production time (at least for 'low significance' errors)? Or, are these just for testing code, and I should better handle all errors each time?

Trapeze

I created a github issue to ask about the intention of the assert package. github.com/nodejs/help/issues/3266 – Gabble

Answer 1

You definitively should not keep them in the production environment.

If you google a bit, there are a plethora of alternative approaches to strip out them.

Personally, I'd use the null object pattern by implementing two wrappers in a separate file: the former maps its method directly to the one exported by the module assert, the latter offers empty functions and nothing more.

Thus, at runtime, you can plug in the right one by relying on some global variable previously correctly set, like process.env.mode. Within your files, you'll have only to import the above mentioned module and use it instead of using directly assert.

This way, all around your code you'll never see error-prone stuff like myAssert && myAssert(cond), instead you'll have ever a cleaner and safer myAssert(cond) statement.

It follows a brief example:

// myassert.js
var assert = require('assert');
if('production' === process.env.mode) {
    var nil = function() { };
    module.exports = {
        equal = nil;
        notEqual = nil;
        // all the other functions
    };
} else {
    // a wrapper like that one helps in not polluting the exported object
    module.exports = {
        equal = function(actual, expected, message) {
            assert.equal(actual, expected, message);
        },
        notEqual = function(actual, expected, message) {
            assert.notEqual(actual, expected, message);
        },
        // all the other functions
    }
}


// another_file.js
var assert = require('path_to_myassert/myassert');
// ... your code
assert(true, false);
// ... go on

Softwood

Could you please elaborate on why one should not keep them in the production environment? – Complacence

@SanghyunLee Expressions you use within the assert can be complex and usually you don't want to spend cpu cycle for things you don't want in production. – Softwood

regarding the latter, you mean one could introduce a bug using assert, right? – Complacence

@SanghyunLee A bug? No. Why should one introduce a bug with an assert? Usually the goal is the opposite one!! – Softwood

ahh, okay I misunderstood. So only to reduce the cpu cycles. – Complacence

If you're concerned about reducing cpu cycles in node.js, you're really playing the wrong game... – Hathaway

Re " the null object pattern" - I like this idea, but it doesn't seem sufficient, because the expensive part of assertion is evaluating the expression. That evaluation will happen, even if you pass it to a method that does nothing with it. Need to strip the entire assertion statement out - unless you limit assertions to calculations that are cheap. OTOH, expensive assertions are rare enough that maybe you live with if (DEBUG) assert(...) for those. – Carty

Answer 2

Yes! asserts are good in production code.

1. Asserts allow a developer to document assumptions that the code makes, making code easier to read and maintain.
2. It is better for an assert to fail in production than allow the undefined behaviour that the assert was protecting. When an assert fails you can more easily see the problem and fix it.
3. Knowing your code is working within assumptions is far more valuable than a small performance gain.

I know opinions differ here. I have offered a 'Yes' answer because I am interested to see how people vote.

Ingalls

Answer 3

probably no

ref: When should assertions stay in production code?

Mostly in my code i put error handling function in a separate file , and use same error method everywhere, it mostly depends on logic anyways

like ppl generally forget this

process.on('uncaughtException', function (err) {
  console.log(err);
})

and err==null doesn't hurts , it checks both null and undefined

Adieu

How did you come to this conclusion when most of the answers of the link you referred to say yes? – Complacence