HTTP Agent sockets should not reconnect on error

Closes GH-684.

lib/http.js

@@ -1144,6 +1144,10 @@ Agent.prototype._establishNewConnection = function() {
 
   this.sockets.push(socket);
 
+  // Cycle so the request can be assigned to this new socket.
+  self._cycle();
+  assert(socket._httpMessage);
+
   // Add a parser to the socket.
   var parser = parsers.alloc();
   parser.reinitialize('response');
@@ -1220,6 +1224,7 @@ Agent.prototype._establishNewConnection = function() {
 
   // When the socket closes remove it from the list of available sockets.
   socket.on('close', function() {
+    debug('AGENT socket close');
     // This is really hacky: What if someone issues a request, the server
     // accepts, but then terminates the connection. There is no parse error,
     // there is no socket-level error. How does the user get informed?
@@ -1311,22 +1316,22 @@ Agent.prototype._getConnection = function(host, port, cb) {
 // waiting sockets. If a waiting socket cannot be found, it will
 // start the process of establishing one.
 Agent.prototype._cycle = function() {
-  debug('Agent _cycle sockets=' + this.sockets.length +
+  debug('Agent _cycle sockets=' + this.sockets.length + ' queue=' + this.queue.length);
-        ' queue=' + this.queue.length);
   var self = this;
 
   var first = this.queue[0];
   if (!first) return;
 
-
   var haveConnectingSocket = false;
 
   // First try to find an available socket.
   for (var i = 0; i < this.sockets.length; i++) {
     var socket = this.sockets[i];
     // If the socket doesn't already have a message it's sending out
-    // and the socket is available for writing...
+    // and the socket is available for writing or it's connecting.
-    if (!socket._httpMessage && (socket.writable && socket.readable)) {
+    // In particular this rules out sockets that are closing.
+    if (!socket._httpMessage &&
+        ((socket.writable && socket.readable) || socket._httpConnecting)) {
       debug('Agent found socket, shift');
       // We found an available connection!
       this.queue.shift(); // remove first from queue.

test/common.js

@@ -66,3 +66,10 @@ process.on('exit', function() {
     }
   }
 });
+
+
+// This function allows one two run an HTTP test agaist both HTTPS and
+// normal HTTP modules. This ensures they fit the same API.
+exports.httpTest = function httpTest(cb) {
+};
+

test/simple/test-https-connecting-to-http.js

@@ -0,0 +1,40 @@
+// This tests the situation where you try to connect a https client
+// to an http server. You should get an error and exit.
+var common = require('../common');
+var assert = require('assert');
+var https = require('https');
+var http = require('http');
+
+
+var reqCount = 0;
+var resCount = 0;
+var reqErrorCount = 0;
+var body = 'hello world\n';
+
+
+var server = http.createServer(function (req, res) {
+  reqCount++;
+  console.log('got request');
+  res.writeHead(200, { 'content-type': 'text/plain' });
+  res.end(body);
+});
+
+
+server.listen(common.PORT, function() {
+  var req = https.get({ port: common.PORT }, function(res) {
+    resCount++;
+  });
+
+  req.on('error', function(e) {
+    console.log('Got expected error: ', e.message);
+    server.close();
+    reqErrorCount++;
+  });
+});
+
+
+process.on('exit', function() {
+  assert.equal(0, reqCount);
+  assert.equal(0, resCount);
+  assert.equal(1, reqErrorCount);
+});