Permalink
Browse files

Fix error and timeout handling.

* Move `error` event handlers to `client` object. This is the actual stream which emits the `error` events.
* Clarify how the timeout handler is used
* Close the `client` after receiving the response body, we won't reuse it anyway
* Ignore `close` events on the `client`, we're the only ones closing it
  • Loading branch information...
1 parent 39f5054 commit 91bcfa2e4f639bee0b9753fde6180d60be403054 @novemberborn novemberborn committed Dec 10, 2010
Showing with 19 additions and 18 deletions.
  1. +19 −18 engines/node/lib/http-client.js
View
37 engines/node/lib/http-client.js
@@ -42,6 +42,22 @@ exports.request = function(originalRequest){
var secure = request.protocol.indexOf("s") > -1;
var client = http.createClient(request.port || (secure ? 443 : 80), request.hostname, secure);
+ var timedOut, bodyDeferred;
+ client.on("error", function(error){
+ if(bodyDeferred){
+ bodyDeferred.reject(error);
+ }else{
+ deferred.reject(error);
+ }
+ clearTimeout(timeout);
+ });
+ // Limits the time of sending the request + receiving the response header to 20 seconds.
+ // No timeout is used on the client stream, but we do destroy the stream if a timeout is reached.
+ var timeout = setTimeout(function(){
+ timedOut = true;
+ client.destroy();
+ deferred.reject(new Error("Timeout"));
+ }, 20000);
var requestPath = request.pathname || request.pathInfo || "";
if (request.queryString) {
@@ -50,7 +66,6 @@ exports.request = function(originalRequest){
var req = client.request(request.method || "GET", requestPath, request.headers ||
{host: request.host || request.hostname + (request.port ? ":" + request.port : "")});
- var timedOut;
req.on("response", function (response){
if(timedOut){
return;
@@ -60,7 +75,7 @@ exports.request = function(originalRequest){
buffer.push(block);
};
var buffer = [];
- var bodyDeferred = defer();
+ bodyDeferred = defer();
var body = response.body = LazyArray({
some: function(callback){
buffer.forEach(callback);
@@ -75,26 +90,12 @@ exports.request = function(originalRequest){
});
response.on("end", function(){
bodyDeferred.resolve();
- });
- response.on("error", function(error){
- bodyDeferred.reject(error);
+ // Since we have no connection pooling, let's not pretend to use Keep-Alive
+ client.end();
});
deferred.resolve(response);
clearTimeout(timeout);
});
- var timeout = setTimeout(function(){
- timedOut = true;
- deferred.reject(new Error("Timeout"));
- }, 20000);
- req.on("error", function(error){
- deferred.reject(error);
- });
- req.on("timeout", function(error){
- deferred.reject(error);
- });
- req.on("close", function(error){
- deferred.reject(error);
- });
if(request.body){
return when(request.body.forEach(function(block){
req.write(block);

0 comments on commit 91bcfa2

Please sign in to comment.