fix: allow request timeout bigger than agent timeout (#3146)

.gitignore

@@ -27,3 +27,4 @@ yarn.lock
 .editorconfig
 *clinic-flame*
 *clinic-doctor*
+.nyc_output/

LICENSE

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2017 Alibaba Group Holding Limited and other contributors.
+Copyright (c) 2017-present Alibaba Group Holding Limited and other contributors.
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

config/config.default.js

@@ -238,12 +238,12 @@ module.exports = appInfo => {
    * @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.freeSocketTimeout - 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.freeSocketTimeout - 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.
    */
@@ -257,13 +257,13 @@ module.exports = appInfo => {
     },
     httpAgent: {
       keepAlive: true,
-      freeSocketKeepAliveTimeout: 4000,
+      freeSocketTimeout: 4000,
       maxSockets: Number.MAX_SAFE_INTEGER,
       maxFreeSockets: 256,
     },
     httpsAgent: {
       keepAlive: true,
-      freeSocketKeepAliveTimeout: 4000,
+      freeSocketTimeout: 4000,
       maxSockets: Number.MAX_SAFE_INTEGER,
       maxFreeSockets: 256,
     },

docs/source/en/core/httpclient.md

@@ -295,7 +295,7 @@ exports.httpclient = {
     // default enable http KeepAlive
     keepAlive: true,
     // idle KeepAlive socket can survive for 4 seconds
-    freeSocketKeepAliveTimeout: 4000,
+    freeSocketTimeout: 4000,
     // when sockets have no activity for more than 30s, it will be processed as timeout
     timeout: 30000,
     // maximum number of sockets allow to be created
@@ -308,7 +308,7 @@ exports.httpclient = {
     // default enable https KeepAlive
     keepAlive: true,
     // idle KeepAlive socket can survive for 4 seconds
-    freeSocketKeepAliveTimeout: 4000,
+    freeSocketTimeout: 4000,
     // when sockets have no activity for more than 30s, it will be processed as timeout
     timeout: 30000,
     // maximum number of sockets allow to be created

docs/source/zh-cn/core/httpclient.md

@@ -301,7 +301,7 @@ exports.httpclient = {
     // 默认开启 http KeepAlive 功能
     keepAlive: true,
     // 空闲的 KeepAlive socket 最长可以存活 4 秒
-    freeSocketKeepAliveTimeout: 4000,
+    freeSocketTimeout: 4000,
     // 当 socket 超过 30 秒都没有任何活动，就会被当作超时处理掉
     timeout: 30000,
     // 允许创建的最大 socket 数
@@ -314,7 +314,7 @@ exports.httpclient = {
     // 默认开启 https KeepAlive 功能
     keepAlive: true,
     // 空闲的 KeepAlive socket 最长可以存活 4 秒
-    freeSocketKeepAliveTimeout: 4000,
+    freeSocketTimeout: 4000,
     // 当 socket 超过 30 秒都没有任何活动，就会被当作超时处理掉
     timeout: 30000,
     // 允许创建的最大 socket 数

index.d.ts

@@ -291,7 +291,8 @@ declare module 'egg' {
 
     httpclient: {
       keepAlive: boolean;
-      freeSocketKeepAliveTimeout: number;
+      freeSocketKeepAliveTimeout?: number;
+      freeSocketTimeout: number;
       timeout: number;
       maxSockets: number;
       maxFreeSockets: number;

lib/core/httpclient.js

@@ -5,11 +5,10 @@ const HttpsAgent = require('agentkeepalive').HttpsAgent;
 const urllib = require('urllib');
 const ms = require('humanize-ms');
 
-
 class HttpClient extends urllib.HttpClient {
   constructor(app) {
-    const config = app.config.httpclient;
     normalizeConfig(app);
+    const config = app.config.httpclient;
     super({
       app,
       defaultArgs: config.request,
@@ -50,11 +49,27 @@ function normalizeConfig(app) {
     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;
+  // compatibility httpclient.freeSocketKeepAliveTimeout => httpclient.freeSocketTimeout
+  if (config.freeSocketKeepAliveTimeout && !config.freeSocketTimeout) {
+    config.freeSocketTimeout = config.freeSocketKeepAliveTimeout;
+    delete config.freeSocketKeepAliveTimeout;
   }
+  if (config.freeSocketTimeout) {
+    config.freeSocketTimeout = ms(config.freeSocketTimeout);
+    config.httpAgent.freeSocketTimeout = config.freeSocketTimeout;
+    config.httpsAgent.freeSocketTimeout = config.freeSocketTimeout;
+  } else {
+    // compatibility agent.freeSocketKeepAliveTimeout
+    if (config.httpAgent.freeSocketKeepAliveTimeout && !config.httpAgent.freeSocketTimeout) {
+      config.httpAgent.freeSocketTimeout = config.httpAgent.freeSocketKeepAliveTimeout;
+      delete config.httpAgent.freeSocketKeepAliveTimeout;
+    }
+    if (config.httpsAgent.freeSocketKeepAliveTimeout && !config.httpsAgent.freeSocketTimeout) {
+      config.httpsAgent.freeSocketTimeout = config.httpsAgent.freeSocketKeepAliveTimeout;
+      delete config.httpsAgent.freeSocketKeepAliveTimeout;
+    }
+  }
+
   if (typeof config.maxSockets === 'number') {
     config.httpAgent.maxSockets = config.maxSockets;
     config.httpsAgent.maxSockets = config.maxSockets;

package.json

@@ -18,7 +18,7 @@
     "@types/koa-router": "^7.0.32",
     "@types/urllib": "^2.28.0",
     "accepts": "^1.3.5",
-    "agentkeepalive": "^3.5.1",
+    "agentkeepalive": "^4.0.0",
     "cache-content-type": "^1.0.1",
     "circular-json": "0.5.5",
     "cluster-client": "^2.1.1",
@@ -51,12 +51,13 @@
     "mz": "^2.7.0",
     "on-finished": "^2.3.0",
     "sendmessage": "^1.1.0",
-    "urllib": "^2.30.0",
+    "urllib": "^2.31.1",
     "utility": "^1.15.0",
     "ylru": "^1.2.1"
   },
   "devDependencies": {
     "address": "^1.0.3",
+    "assert-extends": "^1.0.1",
     "autod": "^3.0.1",
     "autod-egg": "^1.1.0",
     "coffee": "^5.1.0",

test/fixtures/apps/context_httpclient_timeout/config/config.default.js

@@ -0,0 +1,9 @@
+'use strict';
+
+exports.keys = 'test key';
+
+exports.httpclient = {
+  httpAgent: {
+    timeout: 1000,
+  },
+};

test/fixtures/apps/context_httpclient_timeout/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "context_httpclient_timeout"
+}

test/lib/core/context_httpclient_timeout.test.js

@@ -0,0 +1,25 @@
+'use strict';
+
+const assert = require('assert-extends');
+const utils = require('../../utils');
+
+describe('test/lib/core/context_httpclient_timeout.test.js', () => {
+  let url;
+  let app;
+
+  before(() => {
+    app = utils.app('apps/context_httpclient_timeout');
+    return app.ready();
+  });
+  before(async () => {
+    url = await utils.startLocalServer();
+  });
+
+  it('should request timeout override agent socket timeout', () => {
+    app.httpclient.agent.options.timeout = 1000;
+    const ctx = app.mockContext();
+    return assert.asyncThrows(async () => {
+      await ctx.httpclient.request(`${url}/timeout`, { timeout: 1500 });
+    }, /ResponseTimeoutError: Response timeout for 1500ms/);
+  });
+});

test/lib/core/httpclient.test.js

@@ -104,8 +104,8 @@ describe('test/lib/core/httpclient.test.js', () => {
     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.freeSocketTimeout === 2000);
+      assert(app.config.httpclient.httpsAgent.freeSocketTimeout === 2000);
 
       assert(app.config.httpclient.httpAgent.maxSockets === 100);
       assert(app.config.httpclient.httpsAgent.maxSockets === 100);
@@ -376,4 +376,45 @@ describe('test/lib/core/httpclient.test.js', () => {
       assert(reqTracers[0].traceId);
     });
   });
+
+  describe('compatibility freeSocketKeepAliveTimeout', () => {
+    it('should convert freeSocketKeepAliveTimeout to freeSocketTimeout', () => {
+      let mockApp = {
+        config: {
+          httpclient: {
+            request: {},
+            freeSocketKeepAliveTimeout: 1000,
+            httpAgent: {},
+            httpsAgent: {},
+          },
+        },
+      };
+      let client = new Httpclient(mockApp);
+      assert(client);
+      assert(mockApp.config.httpclient.freeSocketTimeout === 1000);
+      assert(!mockApp.config.httpclient.freeSocketKeepAliveTimeout);
+      assert(mockApp.config.httpclient.httpAgent.freeSocketTimeout === 1000);
+      assert(mockApp.config.httpclient.httpsAgent.freeSocketTimeout === 1000);
+
+      mockApp = {
+        config: {
+          httpclient: {
+            request: {},
+            httpAgent: {
+              freeSocketKeepAliveTimeout: 1001,
+            },
+            httpsAgent: {
+              freeSocketKeepAliveTimeout: 1002,
+            },
+          },
+        },
+      };
+      client = new Httpclient(mockApp);
+      assert(client);
+      assert(mockApp.config.httpclient.httpAgent.freeSocketTimeout === 1001);
+      assert(!mockApp.config.httpclient.httpAgent.freeSocketKeepAliveTimeout);
+      assert(mockApp.config.httpclient.httpsAgent.freeSocketTimeout === 1002);
+      assert(!mockApp.config.httpclient.httpsAgent.freeSocketKeepAliveTimeout);
+    });
+  });
 });