feat: switch httpclient to httpclient2 for retry feature (#3606)

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}

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

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);

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;
   },

index.d.ts

@@ -7,13 +7,13 @@ import { Readable } from 'stream';
 import { Socket } from 'net';
 import { IncomingMessage, ServerResponse } from 'http';
 import { EggLogger, EggLoggers, LoggerLevel as EggLoggerLevel, EggContextLogger } from 'egg-logger';
-import { HttpClient2, RequestOptions } from 'urllib';
+import { HttpClient2, RequestOptions2 as RequestOptions } from 'urllib';
 import {
   EggCoreBase,
   FileLoaderOption,
-  EggLoader as CoreLoader, 
-  EggCoreOptions as CoreOptions, 
-  EggLoaderOptions as CoreLoaderOptions, 
+  EggLoader as CoreLoader,
+  EggCoreOptions as CoreOptions,
+  EggLoaderOptions as CoreLoaderOptions,
   BaseContextClass as CoreBaseContextClass,
 } from 'egg-core';
 import EggCookies = require('egg-cookies');
@@ -636,7 +636,7 @@ declare module 'egg' {
 
     truncated: boolean;
   }
-                   
+
   interface GetFileStreamOptions {
     requireFile?: boolean; // required file submit, default is true
     defCharset?: string;
@@ -664,7 +664,7 @@ declare module 'egg' {
   * special properties (e.g: encrypted). So we must remove this property and
   * create our own with the same name.
   * @see https://github.com/eggjs/egg/pull/2958
-  * 
+  *
   * However, the latest version of Koa has "[key: string]: any" on the
   * context, and there'll be a type error for "keyof koa.Context".
   * So we have to directly inherit from "KoaApplication.BaseContext" and

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 = {}) {

lib/application.js

@@ -52,7 +52,7 @@ function escapeHeaderValue(value) {
 class Application extends EggApplication {
 
   /**
-   * @constructor
+   * @class
    * @param {Object} options - see {@link EggApplication}
    */
   constructor(options = {}) {

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) {

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;

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

test/app/extend/context.test.js

@@ -293,8 +293,8 @@ describe('test/app/extend/context.test.js', () => {
         .get('/error')
         .expect(200)
         .expect('hello error');
-      assert(errorHadEmit);
       await sleep(5000);
+      assert(errorHadEmit);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
       assert(/ENOENT: no such file or directory/.test(log));
@@ -359,8 +359,8 @@ describe('test/app/extend/context.test.js', () => {
         .get('/error')
         .expect(200)
         .expect('hello error');
-      assert(errorHadEmit);
       await sleep(5000);
+      assert(errorHadEmit);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
       assert(/ENOENT: no such file or directory/.test(log));

test/fixtures/apps/httpclient-retry/package.json

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

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);
+    });
+  });
 });

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}`);
         }