feat: switch httpclient to httpclient2 for retry feature

eggjs · Apr 10, 2019 · a1e5f7b · a1e5f7b
1 parent e0a1d8f
commit a1e5f7b
Show file tree

Hide file tree

Showing 12 changed files with 79 additions and 14 deletions.
diff --git a/app/extend/context.js b/app/extend/context.js
@@ -39,7 +39,7 @@ const proto = module.exports = {
   /**
    * Shortcut for httpclient.curl
    *
-   * @method Context#curl
+   * @function Context#curl
    * @param {String|Object} url - request url address.
    * @param {Object} [options] - options for request.
    * @return {Object} see {@link ContextHttpClient#curl}

diff --git a/app/extend/helper.js b/app/extend/helper.js
@@ -7,7 +7,7 @@ module.exports = {
 
   /**
    * Generate URL path(without host) for route. Takes the route name and a map of named params.
-   * @method Helper#pathFor
+   * @function Helper#pathFor
    * @param {String} name - Router Name
    * @param {Object} params - Other params
    *
@@ -25,7 +25,7 @@ module.exports = {
 
   /**
    * Generate full URL(with host) for route. Takes the route name and a map of named params.
-   * @method Helper#urlFor
+   * @function Helper#urlFor
    * @param {String} name - Router name
    * @param {Object} params - Other params
    * @example

diff --git a/app/extend/request.js b/app/extend/request.js
@@ -95,9 +95,9 @@ module.exports = {
   },
 
   /**
-   * Get or set the request remote IPv4 address
+   * Get the request remote IPv4 address
    * @member {String} Request#ip
-   * @param {String} ip - IPv4 address
+   * @return {String} IPv4 address
    * @example
    * ```js
    * this.request.ip
@@ -116,6 +116,17 @@ module.exports = {
     return this._ip;
   },
 
+  /**
+   * Set the request remote IPv4 address
+   * @member {String} Request#ip
+   * @param {String} ip - IPv4 address
+   * @example
+   * ```js
+   * this.request.ip
+   * => '127.0.0.1'
+   * => '111.10.2.1'
+   * ```
+   */
   set ip(ip) {
     this._ip = ip;
   },
@@ -226,10 +237,9 @@ module.exports = {
   /**
    * Set query-string as an object.
    *
-   * @method Request#query
+   * @function Request#query
    * @param {Object} obj set querystring and query object for request.
    * @return {void}
-   * @api public
    */
   set query(obj) {
     this.querystring = querystring.stringify(obj);

diff --git a/app/extend/response.js b/app/extend/response.js
@@ -76,7 +76,7 @@ module.exports = {
    * And access log should save 500 not 302,
    * then the `realStatus` can help us find out the real status code.
    * @member {Number} Response#realStatus
-   * @param {Number} status The status code to be set.
+   * @return {Number} The status code to be set.
    */
   get realStatus() {
     if (this[REAL_STATUS]) {
@@ -85,6 +85,16 @@ module.exports = {
     return this.status;
   },
 
+  /**
+   * Set a real status code.
+   *
+   * e.g.: Using 302 status redirect to the global error page
+   * instead of show current 500 status page.
+   * And access log should save 500 not 302,
+   * then the `realStatus` can help us find out the real status code.
+   * @member {Number} Response#realStatus
+   * @param {Number} status The status code to be set.
+   */
   set realStatus(status) {
     this[REAL_STATUS] = status;
   },

diff --git a/lib/agent.js b/lib/agent.js
@@ -14,7 +14,7 @@ const EGG_PATH = Symbol.for('egg#eggPath');
  */
 class Agent extends EggApplication {
   /**
-   * @constructor
+   * @class
    * @param {Object} options - see {@link EggApplication}
    */
   constructor(options = {}) {

diff --git a/lib/application.js b/lib/application.js
@@ -52,7 +52,7 @@ function escapeHeaderValue(value) {
 class Application extends EggApplication {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options - see {@link EggApplication}
    */
   constructor(options = {}) {

diff --git a/lib/core/base_context_logger.js b/lib/core/base_context_logger.js
@@ -5,7 +5,7 @@ const CALL = Symbol('BaseContextLogger#call');
 class BaseContextLogger {
 
   /**
-   * @constructor
+   * @class
    * @param {Context} ctx - context instance
    * @param {String} pathName - class path name
    * @since 1.0.0
@@ -29,6 +29,7 @@ class BaseContextLogger {
 
   /**
    * @member {Function} BaseContextLogger#debug
+   * @param {...any} args - log msg
    * @since 1.2.0
    */
   debug(...args) {
@@ -37,6 +38,7 @@ class BaseContextLogger {
 
   /**
    * @member {Function} BaseContextLogger#info
+   * @param {...any} args - log msg
    * @since 1.2.0
    */
   info(...args) {
@@ -45,6 +47,7 @@ class BaseContextLogger {
 
   /**
    * @member {Function} BaseContextLogger#warn
+   * @param {...any} args - log msg
    * @since 1.2.0
    */
   warn(...args) {
@@ -53,6 +56,7 @@ class BaseContextLogger {
 
   /**
    * @member {Function} BaseContextLogger#error
+   * @param {...any} args - log msg
    * @since 1.2.0
    */
   error(...args) {

diff --git a/lib/core/httpclient.js b/lib/core/httpclient.js
@@ -5,7 +5,7 @@ const HttpsAgent = require('agentkeepalive').HttpsAgent;
 const urllib = require('urllib');
 const ms = require('humanize-ms');
 
-class HttpClient extends urllib.HttpClient {
+class HttpClient extends urllib.HttpClient2 {
   constructor(app) {
     normalizeConfig(app);
     const config = app.config.httpclient;

diff --git a/lib/egg.js b/lib/egg.js
@@ -33,7 +33,7 @@ const CLUSTER_CLIENTS = Symbol.for('egg#clusterClients');
 class EggApplication extends EggCore {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options
    *  - {Object} [type] - type of instance, Agent and Application both extend koa, type can determine what it is.
    *  - {String} [baseDir] - app root dir, default is `process.cwd()`
@@ -508,7 +508,7 @@ class EggApplication extends EggCore {
 
   /**
    * Create egg context
-   * @method EggApplication#createContext
+   * @function EggApplication#createContext
    * @param  {Req} req - node native Request object
    * @param  {Res} res - node native Response object
    * @return {Context} context object

diff --git a/test/fixtures/apps/httpclient-retry/package.json b/test/fixtures/apps/httpclient-retry/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "httpclient-retry"
+}
diff --git a/test/lib/core/httpclient.test.js b/test/lib/core/httpclient.test.js
@@ -417,4 +417,31 @@ describe('test/lib/core/httpclient.test.js', () => {
       assert(!mockApp.config.httpclient.httpsAgent.freeSocketKeepAliveTimeout);
     });
   });
+
+  describe('httpclient retry', () => {
+    let app;
+    before(() => {
+      app = utils.app('apps/httpclient-retry');
+      return app.ready();
+    });
+    after(() => app.close());
+
+    it('should retry when httpclient fail', async () => {
+      let hasRetry = false;
+      const res = await app.httpclient.curl(`${url}/retry`, {
+        retry: 1,
+        retryDelay: 100,
+        isRetry(res) {
+          const shouldRetry = res.status >= 500;
+          if (shouldRetry) {
+            hasRetry = true;
+          }
+          return shouldRetry;
+        },
+      });
+
+      assert(hasRetry);
+      assert(res.status === 200);
+    });
+  });
 });
diff --git a/test/utils.js b/test/utils.js
@@ -50,6 +50,7 @@ exports.startLocalServer = () => {
     if (localServer) {
       return resolve('http://127.0.0.1:' + localServer.address().port);
     }
+    let retry = false;
     localServer = http.createServer((req, res) => {
       req.resume();
       req.on('end', () => {
@@ -62,6 +63,16 @@ exports.startLocalServer = () => {
             res.end(`${req.method} ${req.url}`);
           }, 10000);
           return;
+        } else if (req.url === '/retry') {
+          if (!retry) {
+            retry = true;
+            res.statusCode = 500;
+            res.end();
+          } else {
+            res.statusCode = 200;
+            res.end('retry suc');
+          }
+          return;
         } else {
           res.end(`${req.method} ${req.url}`);
         }