Skip to content

Commit

Permalink
feat: switch httpclient to httpclient2 for retry feature (#3606)
Browse files Browse the repository at this point in the history
  • Loading branch information
atian25 committed Apr 12, 2019
1 parent 0fa9bdb commit 1334825
Show file tree
Hide file tree
Showing 14 changed files with 198 additions and 42 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
*
* @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}
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.
* @method Helper#pathFor
* @function 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.
* @method Helper#urlFor
* @function Helper#urlFor
* @param {String} name - Router name
* @param {Object} params - Other params
* @example
Expand Down
18 changes: 14 additions & 4 deletions app/extend/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,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
Expand All @@ -122,6 +122,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;
},
Expand Down Expand Up @@ -232,10 +243,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);
Expand Down
12 changes: 11 additions & 1 deletion 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
* @param {Number} status The status code to be set.
* @return {Number} The status code to be set.
*/
get realStatus() {
if (this[REAL_STATUS]) {
Expand All @@ -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;
},
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, 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');
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 {
/**
* @constructor
* @class
* @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 {

/**
* @constructor
* @class
* @param {Object} options - see {@link EggApplication}
*/
constructor(options = {}) {
Expand Down
6 changes: 5 additions & 1 deletion 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 {

/**
* @constructor
* @class
* @param {Context} ctx - context instance
* @param {String} pathName - class path name
* @since 1.0.0
Expand All @@ -29,6 +29,7 @@ class BaseContextLogger {

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

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

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

/**
* @member {Function} BaseContextLogger#error
* @param {...any} args - log msg
* @since 1.2.0
*/
error(...args) {
Expand Down
21 changes: 0 additions & 21 deletions lib/core/dnscache_httpclient.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,6 @@ class DNSCacheHttpClient extends HttpClient {
})();
}

curl(url, args, callback) {
return this.request(url, args, callback);
}

requestThunk(url, args) {
this.app.deprecate('[dnscache_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 [DNSLOOKUP](url, args) {
const parsed = typeof url === 'string' ? urlparse(url) : url;
// hostname must exists
Expand Down
34 changes: 32 additions & 2 deletions 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.HttpClient {
class HttpClient extends urllib.HttpClient2 {
constructor(app) {
normalizeConfig(app);
const config = app.config.httpclient;
Expand All @@ -32,7 +32,37 @@ class HttpClient extends urllib.HttpClient {
args.tracer = args.tracer || this.app.tracer;
}

return super.request(url, args, callback);
// the callback style
if (callback) {
this.app.deprecate('[httpclient] We now support async for this function, so callback isn\'t recommended.');
return super.request(url, args)
.then(result => process.nextTick(() => callback(null, result.data, result.res)))
.catch(err => process.nextTick(() => callback(err)));
}

// the Promise style
return super.request(url, args);
}

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

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 {

/**
* @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()`
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/apps/httpclient-retry/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "httpclient-retry"
}

0 comments on commit 1334825

Please sign in to comment.