Better handling of timeouts

node-js-libs · Nov 24, 2010 · ff5f549 · ff5f549
1 parent 26262a6
commit ff5f549
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 22 deletions.
diff --git a/lib/node.io/job.js b/lib/node.io/job.js
@@ -177,8 +177,11 @@ Job.prototype.emit = function () {
     } else {
 
         //Set a timeout for each method if the `timeout` op is set
-        if (this.options.timeout) {
+        if (this.options.timeout && next === 'run') {
             this.js_timeout = setTimeout(function () {
+                if (self.isComplete) {
+                    return;
+                }
                 self.fail(self.instance_input, 'timeout');
                 self.isComplete = true;
             }, this.options.timeout * 1000);

diff --git a/lib/node.io/request.js b/lib/node.io/request.js
@@ -94,38 +94,66 @@ Job.prototype.doRequest = function (method, resource, body, headers, callback, p
         request.write(body);
     }
 
+    //This method is called on each event. If the instance is already complete (i.e. timed out), destroy the request
+    var destroyIfComplete = function () {
+        if (self.isComplete) {
+            //Which is it? :s
+            request.socket.destroy();
+            request.destroy();
+            host.destroy();
+            return;
+        }
+    };
+
     //Watch for errors
     host.addListener('error', function (connectionException) {
-        if (connectionException.errno === 111) {
-            self.fail(self.instance_input, 'ECONNREFUSED');
-        } else {
-            self.fail(self.instance_input, 'connection');
+        if (self.isComplete) {
+            return;
+        }
+        var err;
+        switch (connectionException.errno) {
+        case 111:
+            err = 'ECONNREFUSED';
+            break;
+        case 104:
+            err = 'ECONNRESET';
+            break;
+        case 12:
+            err = 'ETIMEOUT';
+            break;
+        case 11:
+            err = 'DNSFAIL';
+            break;
+        case 4:
+            err = 'ENOTFOUND';
+            break;
+        case 1:
+            err = 'EPERM';
+            break;
+        default:
+            err = 'connection';
+            break;
         }
-        //Which is it? :s
-        request.socket.destroy();
-        request.destroy();
-        host.destroy();
+        self.fail(self.instance_input, err);
+        self.isComplete = true;
+        destroyIfComplete();
     });
 
-    //Set a timeout if the `timeout` option is set
-    var response_timeout;
-    self.cancelPreviousTimeouts();
-    if (this.options.timeout) {
-        response_timeout = setTimeout(function() {
-            clearTimeout(response_timeout);
-            //Which is it? :s
-            request.socket.destroy();
-            request.destroy();
-            host.destroy();
+    //Set a timeout if the `timeout` option is set. Redirects do not reset the timeout
+    if (this.options.timeout && redirects === 0) {
+        self.cancelPreviousTimeouts();
+        self.js_timeout = setTimeout(function() {
+            if (!self.isComplete) {
+                return;
+            }
             self.fail(self.instance_input, 'timeout');
             self.isComplete = true;
+            destroyIfComplete();
         }, this.options.timeout * 1000);
     }
 
     request.on('response', function (response) {
-        if (response_timeout) {
-            clearTimeout(response_timeout);
-        }
+        destroyIfComplete();
 
         response.setEncoding('utf8');
 
@@ -156,10 +184,13 @@ Job.prototype.doRequest = function (method, resource, body, headers, callback, p
 
         var body = '';
         response.on('data', function (chunk) { 
+            destroyIfComplete();
+
             body = body + chunk; 
         });
 
         response.on('end', function () {
+            destroyIfComplete();
 
             var parse_callback = function (err, data) {
                 callback(null, data, response.headers);