From d0c11435fcf7d7a8484bfea7f70a38f62a9c5db8 Mon Sep 17 00:00:00 2001 From: dead-horse Date: Fri, 12 Apr 2019 14:41:10 +0800 Subject: [PATCH] Revert "feat: switch httpclient to httpclient2 for retry feature (#3606)" This reverts commit eead318625347bb9de8f9d7ffc6fae5ae1b33901. --- app/extend/context.js | 2 +- app/extend/helper.js | 4 +-- app/extend/request.js | 18 +++---------- app/extend/response.js | 12 +-------- index.d.ts | 12 ++++----- lib/agent.js | 2 +- lib/application.js | 2 +- lib/core/base_context_logger.js | 6 +---- lib/core/httpclient.js | 2 +- lib/egg.js | 4 +-- test/app/extend/context.test.js | 4 +-- .../apps/httpclient-retry/package.json | 3 --- test/lib/core/httpclient.test.js | 27 ------------------- test/utils.js | 11 -------- 14 files changed, 22 insertions(+), 87 deletions(-) delete mode 100644 test/fixtures/apps/httpclient-retry/package.json diff --git a/app/extend/context.js b/app/extend/context.js index 87b0b7d9e9..ed8041e471 100644 --- a/app/extend/context.js +++ b/app/extend/context.js @@ -39,7 +39,7 @@ const proto = module.exports = { /** * Shortcut for httpclient.curl * - * @function Context#curl + * @method 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 index 3120944d25..90e0d8064c 100644 --- 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. - * @function Helper#pathFor + * @method 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. - * @function Helper#urlFor + * @method 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 index 109810f536..310d7bb76b 100644 --- a/app/extend/request.js +++ b/app/extend/request.js @@ -101,9 +101,9 @@ module.exports = { }, /** - * Get the request remote IPv4 address + * Get or set the request remote IPv4 address * @member {String} Request#ip - * @return {String} IPv4 address + * @param {String} ip - IPv4 address * @example * ```js * this.request.ip @@ -122,17 +122,6 @@ 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; }, @@ -243,9 +232,10 @@ module.exports = { /** * Set query-string as an object. * - * @function Request#query + * @method 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 index 643d53f610..4695a46697 100644 --- 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 - * @return {Number} The status code to be set. + * @param {Number} status The status code to be set. */ get realStatus() { if (this[REAL_STATUS]) { @@ -85,16 +85,6 @@ 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/index.d.ts b/index.d.ts index 5193b52335..1997f2b5a6 100644 --- a/index.d.ts +++ b/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, RequestOptions2 as RequestOptions } from 'urllib'; +import { HttpClient2, 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 diff --git a/lib/agent.js b/lib/agent.js index 5dbbb8d106..6259523f14 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -14,7 +14,7 @@ const EGG_PATH = Symbol.for('egg#eggPath'); */ class Agent extends EggApplication { /** - * @class + * @constructor * @param {Object} options - see {@link EggApplication} */ constructor(options = {}) { diff --git a/lib/application.js b/lib/application.js index 75cff1f420..4f34c42261 100644 --- a/lib/application.js +++ b/lib/application.js @@ -52,7 +52,7 @@ function escapeHeaderValue(value) { class Application extends EggApplication { /** - * @class + * @constructor * @param {Object} options - see {@link EggApplication} */ constructor(options = {}) { diff --git a/lib/core/base_context_logger.js b/lib/core/base_context_logger.js index 64baff541d..f4a8a3b551 100644 --- 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 { /** - * @class + * @constructor * @param {Context} ctx - context instance * @param {String} pathName - class path name * @since 1.0.0 @@ -29,7 +29,6 @@ class BaseContextLogger { /** * @member {Function} BaseContextLogger#debug - * @param {...any} args - log msg * @since 1.2.0 */ debug(...args) { @@ -38,7 +37,6 @@ class BaseContextLogger { /** * @member {Function} BaseContextLogger#info - * @param {...any} args - log msg * @since 1.2.0 */ info(...args) { @@ -47,7 +45,6 @@ class BaseContextLogger { /** * @member {Function} BaseContextLogger#warn - * @param {...any} args - log msg * @since 1.2.0 */ warn(...args) { @@ -56,7 +53,6 @@ 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 index 4e1400b0c0..2ed61b044c 100644 --- 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.HttpClient2 { +class HttpClient extends urllib.HttpClient { constructor(app) { normalizeConfig(app); const config = app.config.httpclient; diff --git a/lib/egg.js b/lib/egg.js index b4421e4654..96c978b534 100644 --- a/lib/egg.js +++ b/lib/egg.js @@ -33,7 +33,7 @@ const CLUSTER_CLIENTS = Symbol.for('egg#clusterClients'); class EggApplication extends EggCore { /** - * @class + * @constructor * @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 - * @function EggApplication#createContext + * @method EggApplication#createContext * @param {Req} req - node native Request object * @param {Res} res - node native Response object * @return {Context} context object diff --git a/test/app/extend/context.test.js b/test/app/extend/context.test.js index 5cf7267112..8d79b0dd94 100644 --- a/test/app/extend/context.test.js +++ b/test/app/extend/context.test.js @@ -293,8 +293,8 @@ describe('test/app/extend/context.test.js', () => { .get('/error') .expect(200) .expect('hello error'); - await sleep(5000); assert(errorHadEmit); + await sleep(5000); 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'); - await sleep(5000); assert(errorHadEmit); + await sleep(5000); 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)); diff --git a/test/fixtures/apps/httpclient-retry/package.json b/test/fixtures/apps/httpclient-retry/package.json deleted file mode 100644 index eb178c25f0..0000000000 --- a/test/fixtures/apps/httpclient-retry/package.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "name": "httpclient-retry" -} \ No newline at end of file diff --git a/test/lib/core/httpclient.test.js b/test/lib/core/httpclient.test.js index c5787b4b6a..dfd99a4d37 100644 --- a/test/lib/core/httpclient.test.js +++ b/test/lib/core/httpclient.test.js @@ -417,31 +417,4 @@ 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 index 4a3b2f9b3b..8f5fac1a9e 100644 --- a/test/utils.js +++ b/test/utils.js @@ -50,7 +50,6 @@ 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', () => { @@ -63,16 +62,6 @@ 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}`); }