Skip to content

Commit

Permalink
Revert "feat: switch httpclient to httpclient2 for retry feature (#3606
Browse files Browse the repository at this point in the history
…)"

This reverts commit eead318.
  • Loading branch information
dead-horse committed Apr 12, 2019
1 parent fb2c8df commit d0c1143
Show file tree
Hide file tree
Showing 14 changed files with 22 additions and 87 deletions.
2 changes: 1 addition & 1 deletion app/extend/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
4 changes: 2 additions & 2 deletions app/extend/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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
Expand Down
18 changes: 4 additions & 14 deletions app/extend/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
},
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 1 addition & 11 deletions app/extend/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand All @@ -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;
},
Expand Down
12 changes: 6 additions & 6 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -636,7 +636,7 @@ declare module 'egg' {

truncated: boolean;
}

interface GetFileStreamOptions {
requireFile?: boolean; // required file submit, default is true
defCharset?: string;
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}) {
Expand Down
2 changes: 1 addition & 1 deletion lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function escapeHeaderValue(value) {
class Application extends EggApplication {

/**
* @class
* @constructor
* @param {Object} options - see {@link EggApplication}
*/
constructor(options = {}) {
Expand Down
6 changes: 1 addition & 5 deletions lib/core/base_context_logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,7 +29,6 @@ class BaseContextLogger {

/**
* @member {Function} BaseContextLogger#debug
* @param {...any} args - log msg
* @since 1.2.0
*/
debug(...args) {
Expand All @@ -38,7 +37,6 @@ class BaseContextLogger {

/**
* @member {Function} BaseContextLogger#info
* @param {...any} args - log msg
* @since 1.2.0
*/
info(...args) {
Expand All @@ -47,7 +45,6 @@ class BaseContextLogger {

/**
* @member {Function} BaseContextLogger#warn
* @param {...any} args - log msg
* @since 1.2.0
*/
warn(...args) {
Expand All @@ -56,7 +53,6 @@ class BaseContextLogger {

/**
* @member {Function} BaseContextLogger#error
* @param {...any} args - log msg
* @since 1.2.0
*/
error(...args) {
Expand Down
2 changes: 1 addition & 1 deletion lib/core/httpclient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/egg.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()`
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/app/extend/context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
3 changes: 0 additions & 3 deletions test/fixtures/apps/httpclient-retry/package.json

This file was deleted.

27 changes: 0 additions & 27 deletions test/lib/core/httpclient.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
11 changes: 0 additions & 11 deletions test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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}`);
}
Expand Down

0 comments on commit d0c1143

Please sign in to comment.