hacky work around socket hangups on http requests

tshinnic · Jan 21, 2011 · f465949 · f465949
1 parent d89454e
commit f465949
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 49 deletions.
diff --git a/lib/http.js b/lib/http.js
@@ -965,15 +965,18 @@ Agent.prototype._establishNewConnection = function() {
 
   socket.on('error', function(err) {
     debug("AGENT SOCKET ERROR: " + err.message);
+    var req; 
     if (socket._httpMessage) {
-      socket._httpMessage.emit('error', err);
+      req = socket._httpMessage
     } else if (self.queue.length) {
-      var req = self.queue.shift();
-      if (req) req.emit('error', err);
+      req = self.queue.shift();
     } else {
       // No requests on queue? Where is the request
       assert(0);
     }
+
+    req.emit('error', err);
+    req._hadError = true; // hacky
   });
 
   socket.ondata = function(d, start, end) {
@@ -987,6 +990,8 @@ Agent.prototype._establishNewConnection = function() {
       socket.onend = null;
 
       var res = parser.incoming;
+      assert(socket._httpMessage);
+      socket._httpMessage.res = res;
 
       // This is start + byteParsed + 1 due to the error of getting \n
       // in the upgradeHead from the closing lines of the headers
@@ -1019,6 +1024,20 @@ Agent.prototype._establishNewConnection = function() {
 
   // When the socket closes remove it from the list of available sockets.
   socket.on('close', function() {
+    // 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?
+    // We check to see if the socket has a request, if so if it has a
+    // response (meaning that it emitted a 'response' event). If the socket
+    // has a request but no response and it never emitted an error event:
+    // THEN we need to trigger it manually.
+    // There must be a better way to do this.
+    if (socket._httpMessage &&
+        !socket._httpMessage.res &&
+        !socket._httpMessage._hadError) {
+      socket._httpMessage.emit('error', new Error('socket hang up'));
+    }
+
     self._removeSocket(socket);
     // unref the parser for easy gc
     parsers.free(parser);
@@ -1254,49 +1273,26 @@ exports.cat = function(url, encoding_, headers_) {
 
   var url = require('url').parse(url);
 
-  var hasHost = false;
-  if (Array.isArray(headers)) {
-    for (var i = 0, l = headers.length; i < l; i++) {
-      if (headers[i][0].toLowerCase() === 'host') {
-        hasHost = true;
-        break;
-      }
-    }
-  } else if (typeof headers === 'Object') {
-    var keys = Object.keys(headers);
-    for (var i = 0, l = keys.length; i < l; i++) {
-      var key = keys[i];
-      if (key.toLowerCase() == 'host') {
-        hasHost = true;
-        break;
-      }
-    }
-  }
-  if (!hasHost) headers['Host'] = url.hostname;
+  var options = {
+    method: 'GET',
+    port: url.port || 80,
+    host: url.hostname,
+    headers: headers,
+    path: (url.pathname || '/') + (url.search || '') + (url.hash || '')
+  };
 
   var content = '';
-
-  var client = exports.createClient(url.port || 80, url.hostname);
-  var req = client.request((url.pathname || '/') +
-                           (url.search || '') +
-                           (url.hash || ''),
-                           headers);
-
-  if (url.protocol == 'https:') {
-    client.https = true;
-  }
-
   var callbackSent = false;
 
-  req.addListener('response', function(res) {
+  var req = exports.request(options, function(res) {
     if (res.statusCode < 200 || res.statusCode >= 300) {
       if (callback && !callbackSent) {
         callback(res.statusCode);
         callbackSent = true;
       }
-      client.end();
       return;
     }
+
     res.setEncoding(encoding);
     res.addListener('data', function(chunk) { content += chunk; });
     res.addListener('end', function() {
@@ -1306,19 +1302,13 @@ exports.cat = function(url, encoding_, headers_) {
       }
     });
   });
+  req.end();
 
-  client.addListener('error', function(err) {
-    if (callback && !callbackSent) {
-      callback(err);
-      callbackSent = true;
-    }
-  });
 
-  client.addListener('close', function() {
+  req.on('error', function(err) {
     if (callback && !callbackSent) {
-      callback(new Error('Connection closed unexpectedly'));
+      callback(err);
       callbackSent = true;
     }
   });
-  req.end();
 };
diff --git a/test/simple/test-http-set-timeout.js b/test/simple/test-http-set-timeout.js
@@ -7,6 +7,7 @@ var server = http.createServer(function(req, res) {
   req.connection.setTimeout(500);
 
   req.connection.addListener('timeout', function() {
+    req.connection.destroy();
     common.debug('TIMEOUT');
     server.close();
   });
@@ -19,9 +20,10 @@ server.listen(common.PORT, function() {
     throw new Error('Timeout was not sucessful');
   }, 2000);
 
-  http.cat('http://localhost:' + common.PORT + '/', 'utf8',
-           function(err, content) {
-             clearTimeout(errorTimer);
-             console.log('HTTP REQUEST COMPLETE (this is good)');
-           });
+  var url = 'http://localhost:' + common.PORT + '/';
+
+  http.cat(url, 'utf8', function(err, content) {
+    clearTimeout(errorTimer);
+    console.log('HTTP REQUEST COMPLETE (this is good)');
+  });
 });