set() call without a callback function fails silently

[bdunavant]

Providing a callback (while best to do) is typically not required. Not providing one with a set() call causes Utils.validateArg to fail. However, since validateArg only reports errors using the provided callback (which does not exist) this silently fails and does nothing. This causes great consternation to a programmer. Especially when the previously failing call to set() works just fine when you are debugging it with a callback to look at the error and find there is none and it works as expected.

This is a very poor failure situation since it should be working fine, yet nothing is getting cached, so you keep falling back to database and only notice because you, thankfully, are smart enough to have monitoring on your cache-miss rates.

You can verify this using the following test script:

var Memcached = require('memcached');
var MEMCACHED_PARAMS = { "servers": [ "127.0.0.1:11211" ], "options": {
            "timeout": 200,
            "retry": 10,
            "retries": 2,
            "poolSize": 20
        }
};
var mc = new Memcached( MEMCACHED_PARAMS.servers, MEMCACHED_PARAMS.options);

mc.set('foo', 1, 1000, function(err, result) {
    if(err) { console.log(err); return err; }
    mc.get('foo', function(err, result) {
        if(err) { console.log(err); return err; }
        console.log('SUCCESS 1: got inital set of ' + result );
        mc.set('foo', 2, 1000);
        console.log('called set without callback to 2');
    });
});

function checkSet() {
    mc.get('foo', function(err, result) {
        if(err) { console.log(err); return err; }
        if( result == 1 ) {
            console.log('FAIL: result is still 1');
        } else if( result == 2 ) {
            console.log('SUCCESS 2');
        } else {
            console.log('FAIL: something is really screwed up.  got: ' + result);
        }
    });
}

setTimeout(checkSet, 2000);

[bdunavant]

My belief is that the correct behavior here is to allow the set() call to attempt to complete as one would expect. Any errors in that call will continue to be silently ignored, but the simple lack of a callback should not be an error. If it is required, it should die horribly instead of continuing IMO.

I will provide a pull request with such.

[jonface]

@bdunavant Thanks for this tip, I was just about to punch a small woodland animal.