Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "feat: switch httpclient to httpclient2 for retry feature (#36… #3622

Merged
merged 2 commits into from Apr 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/extend/context.js
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
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
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
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
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
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
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
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
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
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
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
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
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