swap out forever-agent with agentkeepalive (#196)

package.json

@@ -95,8 +95,8 @@
   },
   "license": "Apache-2.0",
   "dependencies": {
+    "agentkeepalive": "^2.2.0",
     "chalk": "^1.0.0",
-    "forever-agent": "^0.6.0",
     "lodash": "^4.12.0",
     "promise": "^7.1.1"
   },
@@ -113,4 +113,4 @@
   "engines": {
     "node": ">=0.8"
   }
-}
+}

src/lib/connectors/_keep_alive_agent.js

@@ -1,48 +0,0 @@
-var ForeverAgent = require('forever-agent');
-var ForeverSSLAgent = require('forever-agent').SSL;
-
-var NativeAgent = require('http').Agent;
-var NativeSSLAgent = require('https').Agent;
-
-var inherits = require('util').inherits;
-var nativeKeepAlive = (function () {
-  var a = new NativeAgent();
-  return !!a.freeSockets;
-}());
-
-function WrapForeverAgent(opts) {
-  ForeverAgent.call(this, opts);
-  var _addRequest = this.addRequest;
-  this.addRequest = function (req, host, port) {
-    req.useChunkedEncodingByDefault = false;
-    _addRequest.call(this, req, host, port);
-  };
-}
-inherits(WrapForeverAgent, ForeverAgent);
-
-function WrapForeverSSLAgent(opts) {
-  ForeverSSLAgent.call(this, opts);
-  var _addRequest = this.addRequest;
-  this.addRequest = function (req, host, port) {
-    req.useChunkedEncodingByDefault = false;
-    _addRequest.call(this, req, host, port);
-  };
-}
-inherits(WrapForeverSSLAgent, ForeverSSLAgent);
-
-function WrapNativeAgent(opts) { NativeAgent.call(this, opts); }
-inherits(WrapNativeAgent, NativeAgent);
-
-function WrapNativeSSLAgent(opts) { NativeSSLAgent.call(this, opts); }
-inherits(WrapNativeSSLAgent, NativeSSLAgent);
-
-
-if (nativeKeepAlive) {
-  module.exports = WrapNativeAgent;
-  module.exports.SSL = WrapNativeSSLAgent;
-} else {
-  module.exports = WrapForeverAgent;
-  module.exports.SSL = WrapForeverSSLAgent;
-}
-
-module.exports.supportsNativeKeepAlive = nativeKeepAlive;

src/lib/connectors/http.js

@@ -13,8 +13,9 @@ var handles = {
   https: require('https')
 };
 var _ = require('../utils');
+var parseUrl = require('url').parse;
 var qs = require('querystring');
-var KeepAliveAgent = require('./_keep_alive_agent');
+var AgentKeepAlive = require('agentkeepalive');
 var ConnectionAbstract = require('../connection');
 var zlib = require('zlib');
 
@@ -65,7 +66,7 @@ HttpConnector.prototype.onStatusSet = _.handler(function (status) {
     _.each(agent.freeSockets, collectSockets);
     _.each(toRemove, function (args) {
       var host = args[0], socket = args[1];
-      agent.removeSocket(socket, host);
+      agent.removeSocket(socket, parseUrl(host));
       socket.destroy();
     });
   }
@@ -79,7 +80,7 @@ HttpConnector.prototype.createAgent = function (config) {
   }
 
   if (config.keepAlive) {
-    Agent = this.useSsl ? KeepAliveAgent.SSL : KeepAliveAgent;
+    Agent = this.useSsl ? AgentKeepAlive.HttpsAgent : AgentKeepAlive;
     this.on('status set', this.bound.onStatusSet);
   }
 
@@ -88,14 +89,6 @@ HttpConnector.prototype.createAgent = function (config) {
 
 HttpConnector.prototype.makeAgentConfig = function (config) {
   var agentConfig = {
-    /*
-     * As HTTP/HTTPS Agent defaults keepAlive to false, in the case where we
-     * desire HTTP keep-alive, we need to set it appropriately. This could be
-     * done in the wrapper, but I don't see any good reason not to simply set
-     * it here. ¯\_(ツ)_/¯
-     *
-     * https://github.com/elastic/elasticsearch-js/issues/107
-     */
     keepAlive: config.keepAlive,
     maxSockets: config.maxSockets,
     minSockets: config.minSockets

test/unit/specs/http_connector.js

@@ -5,9 +5,10 @@ describe('Http Connector', function () {
   var nock = require('nock');
   var sinon = require('sinon');
   var util = require('util');
+  var parseUrl = require('url').parse;
   var http = require('http');
   var https = require('https');
-  var KeepAliveAgent = require('../../../src/lib/connectors/_keep_alive_agent');
+  var AgentKeepAlive = require('agentkeepalive');
 
   var Host = require('../../../src/lib/host');
   var errors = require('../../../src/lib/errors');
@@ -173,7 +174,7 @@ describe('Http Connector', function () {
       con.request({}, function () {
         expect(http.request.callCount).to.be(1);
         expect(https.request.callCount).to.be(0);
-        expect(http.request.lastCall.args[0].agent).to.be.a(KeepAliveAgent);
+        expect(http.request.lastCall.args[0].agent).to.be.a(AgentKeepAlive);
         done();
       });
     });
@@ -183,7 +184,7 @@ describe('Http Connector', function () {
       con.request({}, function () {
         expect(http.request.callCount).to.be(0);
         expect(https.request.callCount).to.be(1);
-        expect(https.request.lastCall.args[0].agent).to.be.a(KeepAliveAgent.SSL);
+        expect(https.request.lastCall.args[0].agent).to.be.a(AgentKeepAlive.HttpsAgent);
         done();
       });
     });
@@ -473,11 +474,6 @@ describe('Http Connector', function () {
   });
 
   describe('Connection cleanup', function () {
-    // skip these tests if native keep alive requests are supported
-    if (KeepAliveAgent.supportsNativeKeepAlive) {
-      return;
-    }
-
     it('destroys any connections created', function (done) {
       this.timeout(5 * 60 * 1000);
       var cp = require('child_process');
@@ -524,7 +520,8 @@ describe('Http Connector', function () {
         { destroy: function () {} },
         { destroy: function () {} }
       ];
-      con.agent.sockets['http://localhost/'] = sockets;
+      var name = con.agent.getName(parseUrl('http://localhost/'));
+      con.agent.sockets[name] = sockets;
       con.setStatus('closed');
       expect(sockets).to.eql([]);
     });