👌 IMPROVE: Drop httpclient callback and thunk style (#5052)

And remove mz and mz-modules deps
eggjs · Oct 31, 2022 · 610a39e · 610a39e
1 parent 3a941d6
commit 610a39e
Show file tree

Hide file tree

Showing 39 changed files with 150 additions and 453 deletions.
diff --git a/lib/core/context_httpclient.js b/lib/core/context_httpclient.js
@@ -1,5 +1,3 @@
-'use strict';
-
 class ContextHttpClient {
   constructor(ctx) {
     this.ctx = ctx;
@@ -14,14 +12,14 @@ class ContextHttpClient {
    * @param {Object} [options] - options for request.
    * @return {Object} see {@link Application#curl}
    */
-  curl(url, options) {
+  async curl(url, options) {
     options = options || {};
     options.ctx = this.ctx;
-    return this.app.curl(url, options);
+    return await this.app.curl(url, options);
   }
 
-  request(url, options) {
-    return this.curl(url, options);
+  async request(url, options) {
+    return await this.curl(url, options);
   }
 }
 

diff --git a/lib/core/dnscache_httpclient.js b/lib/core/dnscache_httpclient.js
@@ -1,6 +1,4 @@
-'use strict';
-
-const dns = require('mz/dns');
+const dns = require('dns').promises;
 const LRU = require('ylru');
 const HttpClient = require('./httpclient');
 const utility = require('utility');
@@ -17,39 +15,13 @@ class DNSCacheHttpClient extends HttpClient {
     this.dnsCache = new LRU(this.app.config.httpclient.dnsCacheMaxLength);
   }
 
-  request(url, args, callback) {
-    // request(url, callback)
-    if (typeof args === 'function') {
-      callback = args;
-      args = null;
-    }
-
-    // the callback style
-    if (callback) {
-      this.app.deprecate('[dnscache_httpclient] We now support async for this function, so callback isn\'t recommended.');
-      // disable dns cache in request by args handle
-      if (args && args.enableDNSCache === false) {
-        super.request(url, args, callback);
-        return;
-      }
-      this[DNSLOOKUP](url, args)
-        .then(result => {
-          return super.request(result.url, result.args);
-        })
-        .then(result => process.nextTick(() => callback(null, result.data, result.res)))
-        .catch(err => process.nextTick(() => callback(err)));
-      return;
+  async request(url, args) {
+    // disable dns cache in request by args handle
+    if (args && args.enableDNSCache === false) {
+      return await super.request(url, args);
     }
-
-    // the Promise style
-    return (async () => {
-      // disable dns cache in request by args handle
-      if (args && args.enableDNSCache === false) {
-        return super.request(url, args);
-      }
-      const result = await this[DNSLOOKUP](url, args);
-      return super.request(result.url, result.args);
-    })();
+    const result = await this[DNSLOOKUP](url, args);
+    return await super.request(result.url, result.args);
   }
 
   async [DNSLOOKUP](url, args) {
@@ -96,7 +68,7 @@ class DNSCacheHttpClient extends HttpClient {
   async [UPDATE_DNS](hostname, args) {
     const logger = args.ctx ? args.ctx.coreLogger : this.app.coreLogger;
     try {
-      const [ address ] = await dns.lookup(hostname, { family: 4 });
+      const { address } = await dns.lookup(hostname, { family: 4 });
       logger.info('[dnscache_httpclient] dns lookup success: %s => %s',
         hostname, address);
       this.dnsCache.set(hostname, { timestamp: Date.now(), ip: address });
@@ -106,7 +78,6 @@ class DNSCacheHttpClient extends HttpClient {
       throw err;
     }
   }
-
 }
 
 module.exports = DNSCacheHttpClient;

diff --git a/lib/core/httpclient.js b/lib/core/httpclient.js
@@ -1,5 +1,3 @@
-'use strict';
-
 const Agent = require('agentkeepalive');
 const HttpsAgent = require('agentkeepalive').HttpsAgent;
 const urllib = require('urllib');
@@ -25,58 +23,26 @@ class HttpClient extends urllib.HttpClient2 {
     this.app = app;
   }
 
-  request(url, args, callback) {
-    if (typeof args === 'function') {
-      callback = args;
-      args = null;
-    }
-
+  async request(url, args) {
     args = args || {};
-
     if (args.ctx && args.ctx.tracer) {
       args.tracer = args.ctx.tracer;
     } else {
       args.tracer = args.tracer || this.app.tracer;
     }
 
-    // the callback style
-    if (callback) {
-      this.app.deprecate('[httpclient] We now support async for this function, so callback isn\'t recommended.');
-      super.request(url, args)
-        .then(result => process.nextTick(() => callback(null, result.data, result.res)))
-        .catch(err => process.nextTick(() => callback(err)));
-      return;
+    try {
+      return await super.request(url, args);
+    } catch (err) {
+      if (err.code === 'ENETUNREACH') {
+        throw HttpClientError.create(err.message, err.code);
+      }
+      throw err;
     }
-
-    // the Promise style
-    return super.request(url, args)
-      .catch(err => {
-        if (err.code === 'ENETUNREACH') {
-          throw HttpClientError.create(err.message, err.code);
-        }
-        throw err;
-      });
-  }
-
-  curl(url, args, callback) {
-    return this.request(url, args, callback);
   }
 
-  requestThunk(url, args) {
-    this.app.deprecate('[httpclient] Please use `request()` instead of `requestThunk()`');
-    return callback => {
-      this.request(url, args, (err, data, res) => {
-        if (err) {
-          return callback(err);
-        }
-        callback(null, {
-          data,
-          status: res.status,
-          headers: res.headers,
-          res,
-        });
-      });
-    };
+  async curl(...args) {
+    return await this.request(...args);
   }
 }
 

diff --git a/lib/core/httpclient_next.js b/lib/core/httpclient_next.js
@@ -1,5 +1,3 @@
-'use strict';
-
 const { HttpClient } = require('urllib-next');
 const ms = require('humanize-ms');
 

diff --git a/lib/egg.js b/lib/egg.js
@@ -280,8 +280,8 @@ class EggApplication extends EggCore {
    * console.log(result.status, result.headers, result.data);
    * ```
    */
-  curl(url, opts) {
-    return this.httpclient.request(url, opts);
+  async curl(url, opts) {
+    return await this.httpclient.request(url, opts);
   }
 
   /**

diff --git a/package.json b/package.json
@@ -48,7 +48,6 @@
     "koa-is-json": "^1.0.0",
     "koa-override": "^3.0.0",
     "ms": "^2.1.3",
-    "mz": "^2.7.0",
     "on-finished": "^2.4.1",
     "sendmessage": "^1.1.0",
     "urllib": "^2.33.0",
@@ -79,8 +78,6 @@
     "jsdoc": "^3.6.11",
     "koa": "^2.13.4",
     "koa-static": "^5.0.0",
-    "mz": "^2.7.0",
-    "mz-modules": "^2.1.0",
     "pedding": "^1.1.0",
     "prettier": "^2.7.1",
     "runscript": "^1.5.3",

diff --git a/site/docs/basics/controller.md b/site/docs/basics/controller.md
@@ -351,7 +351,7 @@ The corresponding backend codes are:
 ```js
 // app/controller/upload.js
 const Controller = require('egg').Controller;
-const fs = require('mz/fs');
+const fs = require('fs/promises');
 
 module.exports = class extends Controller {
   async upload() {
@@ -399,7 +399,7 @@ The corresponding backend codes are:
 ```js
 // app/controller/upload.js
 const Controller = require('egg').Controller;
-const fs = require('mz/fs');
+const fs = require('fs/promises');
 
 module.exports = class extends Controller {
   async upload() {

diff --git a/site/docs/basics/controller.zh-CN.md b/site/docs/basics/controller.zh-CN.md
@@ -351,7 +351,7 @@ exports.multipart = {
 ```js
 // app/controller/upload.js
 const Controller = require('egg').Controller;
-const fs = require('mz/fs');
+const fs = require('fs/promises');
 
 module.exports = class extends Controller {
   async upload() {
@@ -399,7 +399,7 @@ module.exports = class extends Controller {
 ```js
 // app/controller/upload.js
 const Controller = require('egg').Controller;
-const fs = require('mz/fs');
+const fs = require('fs/promises');
 
 module.exports = class extends Controller {
   async upload() {

diff --git a/test/app/extend/application.test.js b/test/app/extend/application.test.js
@@ -1,7 +1,5 @@
-'use strict';
 
 const assert = require('assert');
-const sleep = require('mz-modules/sleep');
 const fs = require('fs');
 const path = require('path');
 const utils = require('../../utils');
@@ -58,7 +56,7 @@ describe('test/app/extend/application.test.js', () => {
     it('should log info when plugin is not ready', async () => {
       app = utils.cluster('apps/notready');
       // it won't be ready, so wait for the timeout
-      await sleep(11000);
+      await utils.sleep(11000);
 
       app.expect('stderr', /\[egg:core:ready_timeout] 10 seconds later a was still unable to finish./);
     });
@@ -181,7 +179,7 @@ describe('test/app/extend/application.test.js', () => {
         .get('/app_background')
         .expect(200)
         .expect('hello app');
-      await sleep(5000);
+      await utils.sleep(5000);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
       assert(/mock background run at app result file size: \d+/.test(log));
@@ -203,7 +201,7 @@ describe('test/app/extend/application.test.js', () => {
     it('should wait for middleware resolution', async () => {
       const ctx = app.createAnonymousContext();
       await app.handleRequest(ctx, async ctx => {
-        await sleep(100);
+        await utils.sleep(100);
         ctx.body = 'middleware resolution';
       });
       assert(ctx.body === 'middleware resolution');

diff --git a/test/app/extend/context.test.js b/test/app/extend/context.test.js
@@ -1,9 +1,6 @@
-'use strict';
-
 const fs = require('fs');
 const path = require('path');
 const mm = require('egg-mock');
-const sleep = require('mz-modules/sleep');
 const assert = require('assert');
 const utils = require('../../utils');
 
@@ -25,7 +22,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/logger?message=foo')
         .expect('logger');
 
-      await sleep(5000);
+      await utils.sleep(5000);
 
       const errorContent = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
       assert(errorContent.includes('nodejs.Error: error foo'));
@@ -57,7 +54,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/logger?message=foo')
         .expect('logger');
 
-      await sleep(5000);
+      await utils.sleep(5000);
 
       const errorContent = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
       assert(errorContent.includes('nodejs.Error: error foo'));
@@ -86,7 +83,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/logger?message=foo')
         .expect('logger');
 
-      await sleep(5000);
+      await utils.sleep(5000);
 
       const errorContent = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
       assert(errorContent.includes('nodejs.Error: error foo'));
@@ -123,7 +120,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/logger')
         .expect(200);
 
-      await sleep(100);
+      await utils.sleep(100);
       const logPath = utils.getFilepath('apps/get-logger/logs/get-logger/a.log');
       assert(
         /\[-\/127.0.0.1\/-\/\d+ms GET \/logger] aaa/.test(fs.readFileSync(logPath, 'utf8'))
@@ -253,7 +250,7 @@ describe('test/app/extend/context.test.js', () => {
         .expect(200)
         .expect('hello');
       await app.backgroundTasksFinished();
-      await sleep(100);
+      await utils.sleep(100);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
       assert(/background run result file size: \d+/.test(log));
@@ -272,7 +269,7 @@ describe('test/app/extend/context.test.js', () => {
         .expect(200)
         .expect('hello');
       await app.backgroundTasksFinished();
-      await sleep(100);
+      await utils.sleep(100);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
       assert(/background run result file size: \d+/.test(log));
@@ -296,7 +293,7 @@ describe('test/app/extend/context.test.js', () => {
         .expect(200)
         .expect('hello error');
       await app.backgroundTasksFinished();
-      await sleep(100);
+      await utils.sleep(100);
       assert(errorHadEmit);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');
@@ -329,7 +326,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/')
         .expect(200)
         .expect('hello');
-      await sleep(5000);
+      await utils.sleep(5000);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
       assert(/background run result file size: \d+/.test(log));
@@ -347,7 +344,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/custom')
         .expect(200)
         .expect('hello');
-      await sleep(5000);
+      await utils.sleep(5000);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8');
       assert(/background run result file size: \d+/.test(log));
@@ -370,7 +367,7 @@ describe('test/app/extend/context.test.js', () => {
         .get('/error')
         .expect(200)
         .expect('hello error');
-      await sleep(5000);
+      await utils.sleep(5000);
       assert(errorHadEmit);
       const logdir = app.config.logger.dir;
       const log = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8');

diff --git a/test/fixtures/apps/agent-app/plugins/mock-client/mock_client.js b/test/fixtures/apps/agent-app/plugins/mock-client/mock_client.js
@@ -1,7 +1,5 @@
-'use strict';
-
 const EventEmitter = require('events').EventEmitter;
-const sleep = require('mz-modules/sleep');
+const { sleep } = require('../../../../../utils');
 
 class MockClient extends EventEmitter {
   constructor(options) {

diff --git a/test/fixtures/apps/app-server-customized-client-error/config/config.default.js b/test/fixtures/apps/app-server-customized-client-error/config/config.default.js
@@ -1,6 +1,4 @@
-'use strict';
-
-const sleep = require('mz-modules/sleep');
+const { sleep } = require('../../../../utils');
 
 exports.keys = 'my keys';