Merge pull request #1715 from calibr/forever-fix2

Fix forever option in node > 0.10 #1709
request · Aug 10, 2015 · 289106d · 289106d
2 parents 30c7df5 + ae403dc
commit 289106d
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 4 deletions.
diff --git a/index.js b/index.js
@@ -88,6 +88,8 @@ function wrapRequestMethod (method, options, requester, verb) {
     var target = {}
     extend(true, target, options, params)
 
+    target.pool = params.pool || options.pool
+
     if (verb) {
       target.method = (verb === 'del' ? 'DELETE' : verb.toUpperCase())
     }

diff --git a/request.js b/request.js
@@ -483,10 +483,9 @@ Request.prototype.init = function (options) {
       if (v.major === 0 && v.minor <= 10) {
         self.agentClass = protocol === 'http:' ? ForeverAgent : ForeverAgent.SSL
       } else {
-        self.agent = new self.httpModule.Agent({
-          keepAlive: true,
-          maxSockets: (options.pool && options.pool.maxSockets) || Infinity
-        })
+        self.agentClass = self.httpModule.Agent
+        self.agentOptions = self.agentOptions || {}
+        self.agentOptions.keepAlive = true
       }
     } else {
       self.agentClass = self.httpModule.Agent

diff --git a/tests/test-pool.js b/tests/test-pool.js
@@ -58,6 +58,88 @@ tape('forever', function(t) {
   })
 })
 
+tape('forever, should use same agent in sequential requests', function(t) {
+  var r = request.defaults({
+    forever: true
+  })
+  var req1 = r('http://localhost:6767')
+  var req2 = r('http://localhost:6767/somepath')
+  req1.abort()
+  req2.abort()
+  if (typeof req1.agent.destroy === 'function') {
+    req1.agent.destroy()
+  }
+  if (typeof req2.agent.destroy === 'function') {
+    req2.agent.destroy()
+  }
+  t.equal(req1.agent, req2.agent)
+  t.end()
+})
+
+tape('forever, should use same agent in sequential requests(with pool.maxSockets)', function(t) {
+  var r = request.defaults({
+    forever: true,
+    pool: {maxSockets: 1024}
+  })
+  var req1 = r('http://localhost:6767')
+  var req2 = r('http://localhost:6767/somepath')
+  req1.abort()
+  req2.abort()
+  if (typeof req1.agent.destroy === 'function') {
+    req1.agent.destroy()
+  }
+  if (typeof req2.agent.destroy === 'function') {
+    req2.agent.destroy()
+  }
+  t.equal(req1.agent.maxSockets, 1024)
+  t.equal(req1.agent, req2.agent)
+  t.end()
+})
+
+tape('forever, should use same agent in request() and request.verb', function(t) {
+  var r = request.defaults({
+    forever: true,
+    pool: {maxSockets: 1024}
+  })
+  var req1 = r('http://localhost:6767')
+  var req2 = r.get('http://localhost:6767')
+  req1.abort()
+  req2.abort()
+  if (typeof req1.agent.destroy === 'function') {
+    req1.agent.destroy()
+  }
+  if (typeof req2.agent.destroy === 'function') {
+    req2.agent.destroy()
+  }
+  t.equal(req1.agent.maxSockets, 1024)
+  t.equal(req1.agent, req2.agent)
+  t.end()
+})
+
+tape('should use different agent if pool option specified', function(t) {
+  var r = request.defaults({
+    forever: true,
+    pool: {maxSockets: 1024}
+  })
+  var req1 = r('http://localhost:6767')
+  var req2 = r.get({
+    url: 'http://localhost:6767',
+    pool: {maxSockets: 20}
+  })
+  req1.abort()
+  req2.abort()
+  if (typeof req1.agent.destroy === 'function') {
+    req1.agent.destroy()
+  }
+  if (typeof req2.agent.destroy === 'function') {
+    req2.agent.destroy()
+  }
+  t.equal(req1.agent.maxSockets, 1024)
+  t.equal(req2.agent.maxSockets, 20)
+  t.notEqual(req1.agent, req2.agent)
+  t.end()
+})
+
 tape('cleanup', function(t) {
   s.close(function() {
     t.end()