Socket.IO handshake module

For the server of which this is part, it makes a request to one of our web servers to validate whether the PHP Session ID is actually valid. The location this._options.url is a URL that, given a PHPSESSID as a cookie, will return the user ID that is attached.

One of the issues that has crept up is the CPU utilization spiking to 100%. While I have seen several solutions to this, one was that failure to call a callback function can cause it. After effectively disabling the following function (I had it simply do callback(null,1); for testing purposes), the CPU utilization issue stopped.

However, I do believe a contributing cause was that the nature of performing an httpd request might not be trapping all of the errors and possibilities and have modified it to look this like:

var http = require('http');
var url = require('url');

function AuthFunc(auth_opts){
    this._options = auth_opts;
    this.checkPHPSession = function(phpsessid, callback){
        var options = {
            host: url.parse(this._options.url).host,
            port: url.parse(this._options.url).port || 80,
            path: url.parse(this._options.url).pathname,
            auth: url.parse(this._options.url).auth,
            headers: {
                'Cookie': this._options.cookie + '=' + phpsessid
            }
        };
        var data = '';
        http.get(options, function(response){           
            response.on('data', function(chunk) {
                data += chunk;
            }).on('end', function(){
                try{
                    var arr = JSON.parse(data);
                }catch(e){
                    callback(e, false);
                }
                callback(null, arr.uid);
            }).on('close', function(){
                callback(true, false);
            }).on('error', function(e){
                callback(e, false);
            });
        }).on('error', function(e){
            callback(e, false);
        });
    }
}

module.exports = AuthFunc;

The code works for my purposes, but I’m not sure if it can handle fringe cases of HTTP request failure as that might be difficult to test at best.

Does this code look like it will always properly call callback in all cases, no matter what is returned from the server? Is this the most proper way to write an HTTP request that guarantees a result callback of some kind?

Interesting question, I would rewrite

try{
    var arr = JSON.parse(data);
}catch(e){
    callback(e, false);
}
callback(null, arr.uid);

as

try{
    var arr = JSON.parse(data);
    callback(null, arr.uid);
}catch(e){
    callback(e, false);
}

Actually, I would probably go even a step further and write it as

try{
    callback(null, JSON.parse(data).uid);
}catch(e){
    callback(e, false);
}

Otherwise in case of an error, you will have 2 callbacks, which could cause all kinds of trouble, perhaps even make the CPU spike to 100% 😉

Furthermore, I would also put a try/catch around http.get itself, if there is a hardcore http layer problem, then you might not even get to 'close' or 'error'.

Then some other minor items are

• Most of the options could be set outside of checkPHPSession, you should do so and prevent the repeated determinations of host, port, etc.
• Why are you checking for 'close', it seems like one more way to potentially have too many calls to the callback function