tiny fix in http.js

[xenyou]

I found a tiny bug in removeSocket method in http.js.
I think this method wants to remove a socket specified as the first argument from this.sockets[name].
But the current implemtntation removes the first socket from this.sockets[name].
shift method of Array class doesn't take an argument. So I'll fix it to use splice method.

[bnoordhuis]

Sounds plausible. Is there a way to demonstrate the bug with a test case?

Tangential: it seems like the result of the first .indexOf() could (and should) be re-used.

cc @mikeal

[mikeal]

good catch, we should take it.

[xenyou]

Sorry, I don't have a good idea with a test case.
Because this method is internal.
I just checked this to insert console.log in http.js like this.

Agent.prototype.removeSocket = function(s, name, host, port) {
  if (this.sockets[name] && this.sockets[name].indexOf(s) !== -1) {
    console.log('I want to remove ' + s.id);
    var ret = this.sockets[name].shift(this.sockets[name].indexOf(s));
    console.log(ret.id + ' removed');
  } else if ....
}

var debugid = 0;
Agent.prototype.createSocket = function(name, host, port) {
  var self = this;
  var s = self.createConnection(port, host, self.options);
  s.id = debugid++;
....
}

And I used this script to make removeSocket called.

var http = require('http');
var options = {
  host: 'www.google.com', port: 80, method: 'GET'
};
var req1 = http.request(options);
var req2 = http.request(options);
var req3 = http.request(options);
req1.end();
req2.end();
req3.end();

[mikeal]

yeah, it's internals, this isn't really a public method.

it's clearly a bug, we should just take the fix like sane people. i also don't think we should write a lot of tests for internals, we should be free to change the internals so long as tests for the public interfaces pass.

[koichik]

@xenyou - I squashed two commits into a single commit, and added the test. Thanks!

@mikeal - I have noticed that Agent.sockets is changed from that it is written to the API docs.
Therefore a test is necessary :-)