fix: make sure config.httpclient.httpAgent.timeout >= 30000 (#1165)

distinguish options: request, httpAgent and httpsAgent on httpclient
eggjs · Jul 19, 2017 · 988b8c8 · 988b8c8
1 parent 894005c
commit 988b8c8
Show file tree

Hide file tree

Showing 10 changed files with 198 additions and 25 deletions.
diff --git a/config/config.default.js b/config/config.default.js
@@ -231,22 +231,40 @@ module.exports = appInfo => {
   /**
    * The option for httpclient
    * @member Config#httpclient
-   * @property {Boolean} keepAlive - Enable http keepalive or not, default is true
-   * @property {Number} freeSocketKeepAliveTimeout - socket keepalive max free time, default is 4000 ms.
-   * @property {Number} timeout - socket max unative time, default is 30000 ms.
-   * @property {Number} maxSockets - max socket number of one host, default is `Number.MAX_SAFE_INTEGER` @ses https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
-   * @property {Number} maxFreeSockets - max free socket number of one host, default is 256.
    * @property {Boolean} enableDNSCache - Enable DNS lookup from local cache or not, default is false.
+   *
+   * @property {Number} request.timeout - httpclient request default timeout, default is 5000 ms.
+   *
+   * @property {Boolean} httpAgent.keepAlive - Enable http agent keepalive or not, default is true
+   * @property {Number} httpAgent.freeSocketKeepAliveTimeout - http agent socket keepalive max free time, default is 4000 ms.
+   * @property {Number} httpAgent.maxSockets - http agent max socket number of one host, default is `Number.MAX_SAFE_INTEGER` @ses https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
+   * @property {Number} httpAgent.maxFreeSockets - http agent max free socket number of one host, default is 256.
+   *
+   * @property {Boolean} httpsAgent.keepAlive - Enable https agent keepalive or not, default is true
+   * @property {Number} httpsAgent.freeSocketKeepAliveTimeout - httpss agent socket keepalive max free time, default is 4000 ms.
+   * @property {Number} httpsAgent.maxSockets - https agent max socket number of one host, default is `Number.MAX_SAFE_INTEGER` @ses https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
+   * @property {Number} httpsAgent.maxFreeSockets - https agent max free socket number of one host, default is 256.
    */
   config.httpclient = {
-    keepAlive: true,
-    freeSocketKeepAliveTimeout: 4000,
-    timeout: 30000,
-    maxSockets: Number.MAX_SAFE_INTEGER,
-    maxFreeSockets: 256,
     enableDNSCache: false,
     dnsCacheMaxLength: 1000,
     dnsCacheMaxAge: 10000,
+
+    request: {
+      timeout: 5000,
+    },
+    httpAgent: {
+      keepAlive: true,
+      freeSocketKeepAliveTimeout: 4000,
+      maxSockets: Number.MAX_SAFE_INTEGER,
+      maxFreeSockets: 256,
+    },
+    httpsAgent: {
+      keepAlive: true,
+      freeSocketKeepAliveTimeout: 4000,
+      maxSockets: Number.MAX_SAFE_INTEGER,
+      maxFreeSockets: 256,
+    },
   };
 
   /**

diff --git a/docs/source/zh-cn/core/httpclient.md b/docs/source/zh-cn/core/httpclient.md
@@ -257,16 +257,6 @@ module.exports = function* stream(ctx) {
 ```js
 // config/config.default.js
 exports.httpclient = {
-  // 默认开启 http/https KeepAlive 功能
-  keepAlive: true,
-  // 空闲的 KeepAlive socket 最长可以存活 4 秒
-  freeSocketKeepAliveTimeout: 4000,
-  // 当 socket 超过 30 秒都没有任何活动，就会被当作超时处理掉
-  timeout: 30000,
-  // 允许创建的最大 socket 数
-  maxSockets: Number.MAX_SAFE_INTEGER,
-  // 最大空闲 socket 数
-  maxFreeSockets: 256,
   // 是否开启本地 DNS 缓存，默认关闭，开启后有两个特性
   // 1. 所有的 DNS 查询都会默认优先使用缓存的，即使 DNS 查询错误也不影响应用
   // 2. 对同一个域名，在 dnsCacheLookupInterval 的间隔内（默认 10s）只会查询一次
@@ -275,6 +265,37 @@ exports.httpclient = {
   dnsCacheLookupInterval: 10000,
   // DNS 同时缓存的最大域名数量，默认 1000
   dnsCacheMaxLength: 1000,
+
+  request: {
+    // 默认 request 超时时间
+    timeout: 3000,
+  },
+
+  httpAgent: {
+    // 默认开启 http KeepAlive 功能
+    keepAlive: true,
+    // 空闲的 KeepAlive socket 最长可以存活 4 秒
+    freeSocketKeepAliveTimeout: 4000,
+    // 当 socket 超过 30 秒都没有任何活动，就会被当作超时处理掉
+    timeout: 30000,
+    // 允许创建的最大 socket 数
+    maxSockets: Number.MAX_SAFE_INTEGER,
+    // 最大空闲 socket 数
+    maxFreeSockets: 256,
+  },
+
+  httpsAgent: {
+    // 默认开启 https KeepAlive 功能
+    keepAlive: true,
+    // 空闲的 KeepAlive socket 最长可以存活 4 秒
+    freeSocketKeepAliveTimeout: 4000,
+    // 当 socket 超过 30 秒都没有任何活动，就会被当作超时处理掉
+    timeout: 30000,
+    // 允许创建的最大 socket 数
+    maxSockets: Number.MAX_SAFE_INTEGER,
+    // 最大空闲 socket 数
+    maxFreeSockets: 256,
+  },
 };
 ```
 

diff --git a/lib/core/httpclient.js b/lib/core/httpclient.js
@@ -3,16 +3,57 @@
 const Agent = require('agentkeepalive');
 const HttpsAgent = require('agentkeepalive').HttpsAgent;
 const urllib = require('urllib');
+const ms = require('humanize-ms');
 
 module.exports = app => {
   const HttpClient = app.config.httpclient.enableDNSCache ?
     require('./dnscache_httpclient') : urllib.HttpClient;
 
   const config = app.config.httpclient;
 
+  // compatibility
+  if (typeof config.keepAlive === 'boolean') {
+    config.httpAgent.keepAlive = config.keepAlive;
+    config.httpsAgent.keepAlive = config.keepAlive;
+  }
+  if (config.timeout) {
+    config.timeout = ms(config.timeout);
+    config.httpAgent.timeout = config.timeout;
+    config.httpsAgent.timeout = config.timeout;
+  }
+  if (config.freeSocketKeepAliveTimeout) {
+    config.freeSocketKeepAliveTimeout = ms(config.freeSocketKeepAliveTimeout);
+    config.httpAgent.freeSocketKeepAliveTimeout = config.freeSocketKeepAliveTimeout;
+    config.httpsAgent.freeSocketKeepAliveTimeout = config.freeSocketKeepAliveTimeout;
+  }
+  if (typeof config.maxSockets === 'number') {
+    config.httpAgent.maxSockets = config.maxSockets;
+    config.httpsAgent.maxSockets = config.maxSockets;
+  }
+  if (typeof config.maxFreeSockets === 'number') {
+    config.httpAgent.maxFreeSockets = config.maxFreeSockets;
+    config.httpsAgent.maxFreeSockets = config.maxFreeSockets;
+  }
+
+  if (config.httpAgent.timeout < 30000) {
+    app.coreLogger.warn('[egg:httpclient] config.httpclient.httpAgent.timeout(%s) can\'t below 30000, auto reset to 30000',
+      config.httpAgent.timeout);
+    config.httpAgent.timeout = 30000;
+  }
+  if (config.httpsAgent.timeout < 30000) {
+    app.coreLogger.warn('[egg:httpclient] config.httpclient.httpsAgent.timeout(%s) can\'t below 30000, auto reset to 30000',
+      config.httpsAgent.timeout);
+    config.httpsAgent.timeout = 30000;
+  }
+
+  if (typeof config.request.timeout === 'string') {
+    config.request.timeout = ms(config.request.timeout);
+  }
+
   return new HttpClient({
     app,
-    agent: new Agent(config),
-    httpsAgent: new HttpsAgent(config),
+    defaultArgs: config.request,
+    agent: new Agent(config.httpAgent),
+    httpsAgent: new HttpsAgent(config.httpsAgent),
   });
 };
diff --git a/package.json b/package.json
@@ -40,13 +40,14 @@
     "egg-watcher": "^2.1.3",
     "extend2": "^1.0.0",
     "graceful": "^1.0.1",
+    "humanize-ms": "^1.2.1",
     "is-type-of": "^1.0.0",
     "koa-bodyparser": "^2.5.0",
     "koa-is-json": "^1.0.0",
     "koa-override": "^2.0.0",
     "mime-types": "^2.1.15",
     "sendmessage": "^1.1.0",
-    "urllib": "^2.22.0",
+    "urllib": "^2.23.0",
     "utility": "^1.12.0",
     "ylru": "^1.1.0"
   },

diff --git a/test/fixtures/apps/httpclient-agent-timeout-3000/config/config.default.js b/test/fixtures/apps/httpclient-agent-timeout-3000/config/config.default.js
@@ -0,0 +1,14 @@
+'use strict';
+
+exports.httpclient = {
+  // agent timeout
+  timeout: '3s',
+  freeSocketKeepAliveTimeout: '2s',
+  maxSockets: 100,
+  maxFreeSockets: 100,
+  keepAlive: false,
+
+  request: {
+    timeout: '10s',
+  },
+};
diff --git a/test/fixtures/apps/httpclient-agent-timeout-3000/package.json b/test/fixtures/apps/httpclient-agent-timeout-3000/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "httpclient-agent-timeout-3000"
+}
diff --git a/test/fixtures/apps/httpclient-request-timeout-100/config/config.default.js b/test/fixtures/apps/httpclient-request-timeout-100/config/config.default.js
@@ -0,0 +1,7 @@
+'use strict';
+
+exports.httpclient = {
+  request: {
+    timeout: 100,
+  },
+};
diff --git a/test/fixtures/apps/httpclient-request-timeout-100/package.json b/test/fixtures/apps/httpclient-request-timeout-100/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "httpclient-request-timeout-100"
+}
diff --git a/test/lib/core/httpclient.test.js b/test/lib/core/httpclient.test.js
@@ -1,7 +1,6 @@
 'use strict';
 
 const assert = require('assert');
-
 const mm = require('egg-mock');
 const createHttpclient = require('../../../lib/core/httpclient');
 const utils = require('../../utils');
@@ -13,7 +12,11 @@ describe('test/lib/core/httpclient.test.js', () => {
   before(() => {
     client = createHttpclient({
       config: {
-        httpclient: {},
+        httpclient: {
+          request: {},
+          httpAgent: {},
+          httpsAgent: {},
+        },
       },
     });
     client.on('request', info => {
@@ -91,4 +94,61 @@ describe('test/lib/core/httpclient.test.js', () => {
       // console.error(e.stack);
     });
   });
+
+  describe('httpclient.httpAgent.timeout < 30000', () => {
+    let app;
+    before(() => {
+      app = utils.app('apps/httpclient-agent-timeout-3000');
+      return app.ready();
+    });
+    after(() => app.close());
+
+    it('should auto reset httpAgent.timeout to 30000', () => {
+      // should access httpclient first
+      assert(app.httpclient);
+      assert(app.config.httpclient.timeout === 3000);
+      assert(app.config.httpclient.httpAgent.timeout === 30000);
+      assert(app.config.httpclient.httpsAgent.timeout === 30000);
+    });
+
+    it('should set request default global timeout to 10s', () => {
+      // should access httpclient first
+      assert(app.httpclient);
+      assert(app.config.httpclient.request.timeout === 10000);
+    });
+
+    it('should convert compatibility options to agent options', () => {
+      // should access httpclient first
+      assert(app.httpclient);
+      assert(app.config.httpclient.httpAgent.freeSocketKeepAliveTimeout === 2000);
+      assert(app.config.httpclient.httpsAgent.freeSocketKeepAliveTimeout === 2000);
+
+      assert(app.config.httpclient.httpAgent.maxSockets === 100);
+      assert(app.config.httpclient.httpsAgent.maxSockets === 100);
+
+      assert(app.config.httpclient.httpAgent.maxFreeSockets === 100);
+      assert(app.config.httpclient.httpsAgent.maxFreeSockets === 100);
+
+      assert(app.config.httpclient.httpAgent.keepAlive === false);
+      assert(app.config.httpclient.httpsAgent.keepAlive === false);
+    });
+  });
+
+  describe('httpclient.request.timeout = 100', () => {
+    let app;
+    before(() => {
+      app = utils.app('apps/httpclient-request-timeout-100');
+      return app.ready();
+    });
+    after(() => app.close());
+
+    it('should set request default global timeout to 100ms', () => {
+      return app.httpclient.curl(`${url}/timeout`)
+        .catch(err => {
+          assert(err);
+          assert(err.name === 'ResponseTimeoutError');
+          assert(err.message.includes('Response timeout for 100ms'));
+        });
+    });
+  });
 });
diff --git a/test/utils.js b/test/utils.js
@@ -39,6 +39,11 @@ exports.startLocalServer = () => {
         if (req.url === '/get_headers') {
           res.setHeader('Content-Type', 'json');
           res.end(JSON.stringify(req.headers));
+        } else if (req.url === '/timeout') {
+          setTimeout(() => {
+            res.end(`${req.method} ${req.url}`);
+          }, 10000);
+          return;
         } else {
           res.end(`${req.method} ${req.url}`);
         }