fix: allow agent set to null (#301)

and add node 11 on ci
node-modules · Nov 13, 2018 · 227618a · 227618a
1 parent 11a6acd
commit 227618a
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 20 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -6,6 +6,7 @@ node_js:
   - '6'
   - '8'
   - '10'
+  - '11'
 env:
   - AGENT_VERSION=2
   - AGENT_VERSION=3
@@ -28,6 +29,10 @@ matrix:
       env: AGENT_VERSION=2
     - node_js: '10'
       env: AGENT_VERSION=2
+    - node_js: '11'
+      env: AGENT_VERSION=2
+    - node_js: '11'
+      env: AGENT_VERSION=3
 before_install:
   - sed -i.bak '/"agentkeepalive":/d' package.json
 script:

diff --git a/azure-pipelines.template.yml b/azure-pipelines.template.yml
@@ -14,6 +14,8 @@ jobs:
         node_version: 8
       node_10:
         node_version: 10
+      node_11:
+        node_version: 11
     maxParallel: 3
   steps:
   - task: NodeTool@0

diff --git a/lib/urllib.js b/lib/urllib.js
@@ -502,7 +502,7 @@ function requestWithCallback(url, args, callback) {
     // only support agentkeepalive module for now
     // agentkeepalive@4: agent.options.freeSocketTimeout
     // agentkeepalive@3: agent.freeSocketKeepAliveTimeout
-    var freeSocketTimeout = agent.options && agent.options.freeSocketTimeout || agent.freeSocketKeepAliveTimeout;
+    var freeSocketTimeout = agent && (agent.options && agent.options.freeSocketTimeout || agent.freeSocketKeepAliveTimeout);
     if (agent && agent.keepAlive && freeSocketTimeout > 0 &&
         statusCode >= 200 && headers.connection === 'keep-alive' && headers['keep-alive']) {
       // adjust freeSocketTimeout on the socket

diff --git a/test/httpclient2.test.js b/test/httpclient2.test.js
@@ -189,6 +189,66 @@ describe('test/httpclient2.test.js', function () {
     }).catch(done);
   });
 
+  it('should support agent = null', function(done) {
+    var client = new HttpClient({
+      httpsAgent: null,
+      agent: null,
+    });
+
+    var isKeepAlive = [];
+    var url = config.npmRegistry + '/pedding/latest';
+    client.on('response', function(info) {
+      isKeepAlive.push(info.res.keepAliveSocket);
+    });
+    client.request(url, {
+      timeout: 25000,
+    }).then(function() {
+      // console.log(result.headers);
+      // sleep a while to make sure socket release to free queue
+      return sleep(1);
+    }).then(function() {
+      return client.request(url, {
+        timeout: 25000,
+      });
+    }).then(function() {
+      // console.log(result.headers);
+      assert(isKeepAlive.length === 2);
+      assert(isKeepAlive[0] === false);
+      assert(isKeepAlive[1] === false);
+      done();
+    }).catch(done);
+  });
+
+  it('should support agent = false', function(done) {
+    var client = new HttpClient({
+      httpsAgent: false,
+      agent: false,
+    });
+
+    var isKeepAlive = [];
+    var url = config.npmRegistry + '/pedding/latest';
+    client.on('response', function(info) {
+      isKeepAlive.push(info.res.keepAliveSocket);
+    });
+    client.request(url, {
+      timeout: 25000,
+    }).then(function() {
+      // console.log(result.headers);
+      // sleep a while to make sure socket release to free queue
+      return sleep(1);
+    }).then(function() {
+      return client.request(url, {
+        timeout: 25000,
+      });
+    }).then(function() {
+      // console.log(result.headers);
+      assert(isKeepAlive.length === 2);
+      assert(isKeepAlive[0] === false);
+      assert(isKeepAlive[1] === false);
+      done();
+    }).catch(done);
+  });
+
   it('should create HttpClient2 with defaultArgs', function(done) {
     var client = new urllib.HttpClient2({
       defaultArgs: {

diff --git a/test/request-with-stream.test.js b/test/request-with-stream.test.js
@@ -3,10 +3,11 @@
 var assert = require('assert');
 var fs = require('fs');
 var path = require('path');
+var pedding = require('pedding');
 var urllib = require('..');
 var server = require('./fixtures/server');
 
-describe('request-with-stream.test.js', function() {
+describe('test/request-with-stream.test.js', function() {
   var isNode012 = /^v0\.12\.\d+$/.test(process.version);
   if (isNode012) {
     return;
@@ -25,6 +26,7 @@ describe('request-with-stream.test.js', function() {
   });
 
   it('should close stream when request timeout', function(done) {
+    done = pedding(2, done);
     var tmpfile = path.join(__dirname, '.tmp.txt');
     var buf = Buffer.alloc && Buffer.alloc(10 * 1024 * 1024) || new Buffer(10 * 1024 * 1024);
     fs.writeFileSync(tmpfile, buf);
@@ -34,45 +36,38 @@ describe('request-with-stream.test.js', function() {
       stream: stream,
       timeout: 1000,
     };
-    var streamClosed = false;
     stream.on('close', function() {
-      streamClosed = true;
       console.log('stream close fired');
+      done();
     });
     urllib.request('http://localhost:' + port + '/block', args, function(err, res) {
       assert(err);
       assert(err.name === 'ResponseTimeoutError');
       assert(err.message.indexOf('timeout for 1000ms') > 0);
       assert(!res);
-      setTimeout(function() {
-        assert(streamClosed);
-        done();
-      }, 100);
+      done();
     });
   });
 
   it('should close writeStream when request timeout', function(done) {
+    done = pedding(2, done);
     var tmpfile = path.join(__dirname, '.tmp.txt');
     var writeStream = fs.createWriteStream(tmpfile);
     var args = {
       method: 'POST',
       writeStream: writeStream,
       timeout: 1000,
     };
-    var streamClosed = false;
     writeStream.on('close', function() {
-      streamClosed = true;
       console.log('writeStream close fired');
+      done();
     });
     urllib.request('http://localhost:' + port + '/response_timeout_10s', args, function(err, res) {
       assert(err);
       assert(err.name === 'ResponseTimeoutError');
       assert(err.message.indexOf('timeout for 1000ms') > 0);
       assert(!res);
-      setTimeout(function() {
-        assert(streamClosed);
-        done();
-      }, 100);
+      done();
     });
   });
 

diff --git a/test/urllib.test.js b/test/urllib.test.js
@@ -75,10 +75,8 @@ describe('test/urllib.test.js', function () {
   });
 
   describe('request()', function () {
-
     it('should request(host-only) work', function(done) {
-      var host = urlutil.parse('http://r.cnpmjs.org').host;
-      urllib.request(host, { timeout: 30000 }, function(err, data, res) {
+      urllib.request('127.0.0.1:' + port, function(err, data, res) {
         assert(!err);
         assert(data instanceof Buffer);
         assert(res.statusCode === 200);
@@ -758,7 +756,7 @@ describe('test/urllib.test.js', function () {
         done = pedding(2, done);
 
         var errCount = 0;
-        urllib.request(host + '/timeout', {agent: agent, timeout: 550}, function (err, data) {
+        urllib.request(host + '/timeout', {agent: agent, timeout: 1000}, function (err, data) {
           assert(!err);
           assert(errCount === 1);
           errCount = -1;
@@ -1073,7 +1071,7 @@ describe('test/urllib.test.js', function () {
 
     it('should end when writeStream is not consumed', function(done) {
       var writeStream = through();
-      urllib.request('https://registry.cnpmjs.org', {
+      urllib.request(host, {
         writeStream: writeStream,
         consumeWriteStream: false,
         timeout: 25000,
@@ -1088,7 +1086,7 @@ describe('test/urllib.test.js', function () {
           content += data;
         })
         .on('end', function() {
-          assert(new Buffer(content).length > 100);
+          assert(content.length > 80);
           done();
         });
       });