feat: mark off connection state and response state

use req socket event to impl this Fixes #53
node-modules · Feb 16, 2015 · a0d36f8 · a0d36f8
1 parent 97e7422
commit a0d36f8
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 85 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,9 +1,10 @@
 language: node_js
 node_js:
-  - '0.10'
-  - '0.11'
-  - '0.12'
-  - 'iojs-1.0'
+  - 'iojs-1.2'
   - 'iojs-1.1'
+  - 'iojs-1.0'
+  - '0.12'
+  - '0.11'
+  - '0.10'
 script: "npm run test-travis"
 after_script: "npm install coveralls@2 && cat ./coverage/lcov.info | coveralls"
diff --git a/appveyor.yml b/appveyor.yml
@@ -1,10 +1,11 @@
 environment:
   matrix:
-    - nodejs_version: "0.10"
-    - nodejs_version: "0.11"
-    # - nodejs_version: "0.12"
+    - nodejs_version: "1.2"
+    - nodejs_version: "1.1"
     - nodejs_version: "1.0"
-    # - nodejs_version: "1.1"
+    - nodejs_version: "0.12"
+    - nodejs_version: "0.11"
+    - nodejs_version: "0.10"
 
 # Install scripts. (runs after repo cloning)
 install:

diff --git a/lib/urllib.js b/lib/urllib.js
@@ -280,6 +280,7 @@ exports.requestWithCallback = function (url, args, callback) {
   var timer = null;
   var __err = null;
   var connnected = false; // socket connected or not
+  var keepAliveSocket = false; // request with keepalive socket
   var responseSize = 0;
   var responseAborted = false;
   var done = function (err, data, res) {
@@ -332,11 +333,13 @@ exports.requestWithCallback = function (url, args, callback) {
       headers: headers,
       size: responseSize,
       aborted: responseAborted,
-      rt: requestUsetime
+      rt: requestUsetime,
+      keepAliveSocket: keepAliveSocket,
     };
 
     if (err) {
       err.message += ', ' + options.method + ' ' + url + ' ' + statusCode
+        + ' (connnected: ' + connnected + ', keepalive socket: ' + keepAliveSocket + ')'
         + '\nheaders: ' + JSON.stringify(headers);
       err.data = data;
       err.path = options.path;
@@ -423,7 +426,6 @@ exports.requestWithCallback = function (url, args, callback) {
   debug('Request#%d %s %s with headers %j, options.path: %s',
     reqId, method, url, options.headers, options.path);
   var req = httplib.request(options, function (res) {
-    connnected = true;
     debug('Request#%d %s `req response` event emit: status %d, headers: %j',
       reqId, url, res.statusCode, res.headers);
 
@@ -503,7 +505,7 @@ exports.requestWithCallback = function (url, args, callback) {
       debug('Request#%d %s: `res end` event emit, total size %d, _dumped: %s',
         reqId, url, responseSize, res._dumped);
 
-      if (__err && connnected) {
+      if (__err) {
         // req.abort() after `res data` event emit.
         return done(__err, body, res);
       }
@@ -577,28 +579,39 @@ exports.requestWithCallback = function (url, args, callback) {
     }
     __err = new Error(msg);
     __err.name = errorName;
-    debug('Request#%d %s %s: %s, connected: %s', reqId, options.path, __err.name, msg, connnected);
+    debug('Request#%d %s %s: %s, connected: %s', reqId, url, __err.name, msg, connnected);
     abortRequest();
   }, timeout);
 
-  // req.on('close', function () {
-  //   debug('Request#%d %s: `req close` event emit', reqId, options.path);
-  // });
+  req.once('socket', function (socket) {
+    // https://github.com/iojs/io.js/blob/v1.x/lib/net.js#L342
+    if (socket.readyState === 'opening') {
+      socket.once('connect', function () {
+        debug('Request#%d %s new socket connected', reqId, url);
+        connnected = true;
+      });
+      return;
+    }
+
+    debug('Request#%d %s reuse socket connected', reqId, url);
+    connnected = true;
+    keepAliveSocket = true;
+  });
 
   req.on('error', function (err) {
     if (err.name === 'Error') {
-      err.name = 'RequestError';
+      err.name = connnected ? 'ResponseError' : 'RequestError';
     }
     err.message += ' (req "error")';
-    debug('Request#%d %s `req error` event emit, %s: %s', reqId, options.path, err.name, err.message);
+    debug('Request#%d %s `req error` event emit, %s: %s', reqId, url, err.name, err.message);
     done(__err || err);
   });
 
   if (writeStream) {
     writeStream.once('error', function (err) {
       err.message += ' (writeStream "error")';
       __err = err;
-      debug('Request#%d %s `writeStream error` event emit, %s: %s', reqId, options.path, err.name, err.message);
+      debug('Request#%d %s `writeStream error` event emit, %s: %s', reqId, url, err.name, err.message);
       abortRequest();
     });
   }
@@ -608,7 +621,7 @@ exports.requestWithCallback = function (url, args, callback) {
     args.stream.once('error', function (err) {
       err.message += ' (stream "error")';
       __err = err;
-      debug('Request#%d %s `readStream error` event emit, %s: %s', reqId, options.path, err.name, err.message);
+      debug('Request#%d %s `readStream error` event emit, %s: %s', reqId, url, err.name, err.message);
       abortRequest();
     });
   } else {

diff --git a/test/urllib.test.js b/test/urllib.test.js
@@ -58,7 +58,8 @@ describe('urllib.test.js', function () {
 
   describe('request()', function () {
     it('should request https success', function (done) {
-      urllib.request('https://www.nodejitsu.com/npm/', {timeout: 10000}, function (err, data, res) {
+      urllib.request('https://iojs.org/dist/v1.2.0/SHASUMS256.txt', {timeout: 10000},
+      function (err, data, res) {
         should.not.exist(err);
         should.ok(Buffer.isBuffer(data));
         res.should.status(200);
@@ -110,18 +111,18 @@ describe('urllib.test.js', function () {
     });
 
     describe('ConnectionTimeoutError and ResponseTimeoutError', function () {
-      it('should 500ms connection timeout', function (done) {
-        urllib.request(host + '/timeout', { timeout: 450 }, function (err, data, res) {
+      it('should connection timeout', function (done) {
+        urllib.request('http://npm.taobao.org', { timeout: 1 }, function (err, data, res) {
           should.exist(err);
           err.name.should.equal('ConnectionTimeoutError');
-          err.message.should.match(/^Request#\d+ timeout for 450ms\, GET http/);
+          err.message.should.match(/^Request#\d+ timeout for 1ms\, GET http/);
           should.not.exist(data);
           should.exist(res);
           done();
         });
       });
 
-      it('should 500ms response timeout', function (done) {
+      it('should response timeout', function (done) {
         urllib.request(host + '/response_timeout', { timeout: 450 }, function (err, data, res) {
           should.exist(err);
           err.name.should.equal('ResponseTimeoutError');
@@ -138,7 +139,7 @@ describe('urllib.test.js', function () {
     it('should socket hang up by res.socket.destroy() before `response` event emit', function (done) {
       urllib.request(host + '/error', function (err, data, res) {
         should.exist(err);
-        err.name.should.equal('RequestError');
+        err.name.should.equal('ResponseError');
         err.stack.should.containEql('socket hang up');
         err.code.should.equal('ECONNRESET');
         should.not.exist(data);
@@ -150,7 +151,6 @@ describe('urllib.test.js', function () {
     it('should socket hang up by req.abort() before `response` event emit', function (done) {
       var req = urllib.request(host + '/timeout', {timeout: 500}, function (err, data, res) {
         should.exist(err);
-        err.name.should.equal('RequestError');
         err.stack.should.containEql('socket hang up');
         err.code.should.equal('ECONNRESET');
         should.not.exist(data);
@@ -165,7 +165,7 @@ describe('urllib.test.js', function () {
     it('should handle server socket end("balabal") will error', function (done) {
       urllib.request(host + '/socket.end.error', function (err, data) {
         should.exist(err);
-        err.name.should.equal('RequestError');
+        err.name.should.equal('ResponseError');
         err.code && err.code.should.equal('HPE_INVALID_CHUNK_SIZE');
         err.message.should.containEql('Parse Error (req "error"), GET http://127.0.0.1:');
         err.bytesParsed.should.equal(2);
@@ -490,30 +490,27 @@ describe('urllib.test.js', function () {
           this.agent = new KeepAliveAgent({
             keepAlive: true,
           });
+          this.httpsAgent = new KeepAliveAgent.HttpsAgent({
+            keepAlive: true
+          });
         });
 
         var urls = [
-          'https://opbeat.com/',
-          'https://www.nodejitsu.com',
-          'https://opbeat.com/about',
-          'https://www.nodejitsu.com/npm/',
-          // 'https://www.npmjs.org/search?q=urllib',
-          // 'http://www.taobao.com/sitemap.php',
-          // 'http://nodejs.org/',
-          // 'http://cnpmjs.org/',
-
-          // 'https://www.npmjs.org/package/urllib',
-          // 'https://www.npmjs.org/',
-          // 'http://www.taobao.com/',
-          // 'http://nodejs.org/docs/latest/api/https.html',
-          // 'http://cnpmjs.org/package/urllib',
+          'http://r.cnpmjs.org/',
+          'https://cnpmjs.org/mirrors/iojs/v1.2.0/SHASUMS256.txt',
+          'https://cnpmjs.org/mirrors/iojs/v1.2.0/SHASUMS256.txt',
+          'https://cnpmjs.org/mirrors/iojs/v1.2.0/SHASUMS256.txt',
+          'http://r.cnpmjs.org/pedding',
+          'https://cnpmjs.org/mirrors/iojs/v1.1.0/SHASUMS256.txt',
         ];
 
-        urls.forEach(function (url) {
+        urls.forEach(function (url, index) {
           it('should use KeepAlive agent request ' + url, function (done) {
             var agent = this.agent;
+            var httpsAgent = this.httpsAgent;
             urllib.request(url, {
               agent: agent,
+              httpsAgent: httpsAgent,
               timeout: 15000,
             }, function (err, data, res) {
               should.not.exist(err);
@@ -522,6 +519,9 @@ describe('urllib.test.js', function () {
                 console.log(res.statusCode, res.headers);
               }
               res.should.have.header('connection', 'keep-alive');
+              if (index >= 2) {
+                res.keepAliveSocket.should.equal(true);
+              }
               done();
             });
           });
@@ -646,14 +646,14 @@ describe('urllib.test.js', function () {
         writeStream: writeStream
       }, function (err, data, res) {
         should.exist(err);
-        err.name.should.equal('RequestError');
+        err.name.should.equal('ResponseError');
         err.stack.should.match(/socket hang up/);
         err.code.should.equal('ECONNRESET');
-        err.message.should.containEql('/error -1\nheaders: {}');
+        err.message.should.containEql('/error -1 (connnected: true, keepalive socket: false)\nheaders: {}');
         err.res.should.equal(res);
         should.not.exist(data);
         should.exist(res);
-        res.should.have.keys('status', 'statusCode', 'headers', 'size', 'rt', 'aborted');
+        res.should.have.keys('status', 'statusCode', 'headers', 'size', 'rt', 'aborted', 'keepAliveSocket');
         done();
       });
     });
@@ -681,26 +681,14 @@ describe('urllib.test.js', function () {
       urllib.request('https://no-exist/fengmk2/urllib', {
         timeout: 10000,
         customResponse: true
-      }, function (err, data, res) {
+      }, function (err) {
+        should.exist(err);
         err.code.should.equal('ENOTFOUND');
         done();
       });
     });
   });
 
-  describe('https request', function () {
-    it('GET github page', function (done) {
-      urllib.request('https://github.com/node-modules/urllib', { timeout: 15000 },
-      function (err, data, res) {
-        should.not.exist(err);
-        data.toString().should.containEql('node-modules/urllib');
-        res.should.status(200);
-        res.should.have.header('content-type', 'text/html; charset=utf-8');
-        done();
-      });
-    });
-  });
-
   describe('application/json content-type request', function () {
     it('should auto convert data to json string', function (done) {
       var params = {
@@ -790,19 +778,21 @@ describe('urllib.test.js', function () {
 
   describe('gzip content', function () {
     it('should auto accept and decode gzip response content', function (done) {
-      urllib.request('https://registry.npmjs.org/byte',
+      urllib.request('http://registry.cnpmjs.org/byte',
         {dataType: 'json', gzip: true, timeout: 10000}, function (err, data, res) {
         should.not.exist(err);
         data.name.should.equal('byte');
-        // res.should.have.header('content-encoding', 'gzip');
-        res.should.have.header('content-type', 'application/json');
+        res.should.have.header('content-encoding', 'gzip');
+        res.should.have.header('content-type', 'application/json; charset=utf-8');
         done();
       });
     });
 
     it('should auto accept and custom decode gzip response content', function (done) {
-      urllib.request('https://www.nodejitsu.com/company/contact/', {
-        dataType: 'json', gzip: true, timeout: 10000,
+      urllib.request('http://registry.cnpmjs.org/byte', {
+        dataType: 'json',
+        gzip: true,
+        timeout: 10000,
         headers: {
           'accept-encoding': 'gzip'
         }
@@ -819,45 +809,33 @@ describe('urllib.test.js', function () {
       });
     });
 
-    it.skip('should redirect and gzip', function (done) {
-      urllib.request('http://dist.u.qiniudn.com/v0.10.1/SHASUMS.txt',
+    it('should redirect and gzip', function (done) {
+      urllib.request('http://cnpmjs.org/pedding',
         {followRedirect: true, gzip: true, timeout: 10000}, function (err, data, res) {
         should.not.exist(err);
-        data.toString().should.containEql('e213170fe5ec7721b31149fba1a7a691c50b5379');
         res.should.status(200);
-        // res.should.have.header('content-encoding', 'gzip');
-        // res.should.have.header('content-type', 'text/plain');
-        done();
-      });
-    });
-
-    it.skip('should not ungzip binary content', function (done) {
-      urllib.request('http://dist.u.qiniudn.com/v0.10.0/node.exp', {gzip: true, timeout: 10000},
-      function (err, data, res) {
-        should.not.exist(err);
-        should.not.exist(res.headers['content-encoding']);
-        res.should.have.header('content-type', 'application/octet-stream');
+        res.should.have.header('content-encoding', 'gzip');
         done();
       });
     });
 
     it('should not return gzip response content', function (done) {
       done = pedding(3, done);
-      urllib.request('https://www.nodejitsu.com/company/contact/', {timeout: 10000},
+      urllib.request('http://cnpmjs.org', {timeout: 10000},
       function (err, data, res) {
         should.not.exist(err);
         should.not.exist(res.headers['content-encoding']);
         done();
       });
 
-      urllib.request('https://www.nodejitsu.com/company/contact/', {gzip: false, timeout: 10000},
+      urllib.request('http://cnpmjs.org', {gzip: false, timeout: 10000},
       function (err, data, res) {
         should.not.exist(err);
         should.not.exist(res.headers['content-encoding']);
         done();
       });
 
-      urllib.request('https://www.nodejitsu.com/company/contact/', {gzip: true, timeout: 10000},
+      urllib.request('http://cnpmjs.org', {gzip: true, timeout: 10000},
       function (err, data, res) {
         should.not.exist(err);
         res.should.have.header('content-encoding', 'gzip');

diff --git a/test/urllib_promise.test.js b/test/urllib_promise.test.js
@@ -30,7 +30,7 @@ describe('urllib_promise.test.js', function () {
       result.res.should.status(200);
       result.res.should.have.header('connection');
       result.res.headers.connection.toLowerCase().should.equal('keep-alive');
-      result.res.should.have.keys('status', 'statusCode', 'headers', 'rt', 'size', 'aborted');
+      result.res.should.have.keys('status', 'statusCode', 'headers', 'rt', 'size', 'aborted', 'keepAliveSocket');
       result.res.status.should.equal(200);
       result.res.rt.should.above(0);
       result.res.size.should.above(0);
@@ -56,7 +56,7 @@ describe('urllib_promise.test.js', function () {
   });
 
   it('should throw error', function (done) {
-    urllib.request('http://127.0.0.1:12312', {
+    urllib.request('http://127.0.0.1:11', {
       data: {
         q: 'foo'
       }
@@ -68,7 +68,7 @@ describe('urllib_promise.test.js', function () {
       err.code.should.equal('ECONNREFUSED');
       err.status.should.equal(-1);
       err.headers.should.eql({});
-      err.res.should.have.keys('status', 'statusCode', 'headers', 'rt', 'size', 'aborted');
+      err.res.should.have.keys('status', 'statusCode', 'headers', 'rt', 'size', 'aborted', 'keepAliveSocket');
       done();
     });
   });