Skip to content

Commit

Permalink
feat: support allowH2 on urllib@4
Browse files Browse the repository at this point in the history
  • Loading branch information
fengmk2 committed Sep 15, 2024
1 parent 550a4f1 commit f9c1e89
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 69 deletions.
4 changes: 2 additions & 2 deletions config/config.default.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ module.exports = appInfo => {
* @property {Number} httpsAgent.freeSocketTimeout - httpss agent socket keepalive max free time, default is 4000 ms.
* @property {Number} httpsAgent.maxSockets - https agent max socket number of one host, default is `Number.MAX_SAFE_INTEGER` @ses https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
* @property {Number} httpsAgent.maxFreeSockets - https agent max free socket number of one host, default is 256.
* @property {Boolean} useHttpClientNext - use urllib@3 HttpClient
* @property {Boolean} allowH2 - Allow to use HTTP2 first, only work on `useHttpClientNext = true`
* @property {Boolean} useHttpClientNext - use urllib@3 HttpClient, default is false
* @property {Boolean} allowH2 - use urllib@4 HttpClient and enable H2, default is false. Only works on Node.js >= 18
*/
config.httpclient = {
enableDNSCache: false,
Expand Down
38 changes: 38 additions & 0 deletions lib/core/httpclient4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const { HttpClient } = require('urllib4');
const ms = require('humanize-ms');

class HttpClient4 extends HttpClient {
constructor(app) {
normalizeConfig(app);
const config = app.config.httpclient;
super({
app,
defaultArgs: config.request,
allowH2: config.allowH2,
});
this.app = app;
}

async request(url, options) {
options = options || {};
if (options.ctx && options.ctx.tracer) {
options.tracer = options.ctx.tracer;

Check warning on line 19 in lib/core/httpclient4.js

View check run for this annotation

Codecov / codecov/patch

lib/core/httpclient4.js#L19

Added line #L19 was not covered by tests
} else {
options.tracer = options.tracer || this.app.tracer;
}
return await super.request(url, options);
}

async curl(...args) {
return await this.request(...args);
}

Check warning on line 28 in lib/core/httpclient4.js

View check run for this annotation

Codecov / codecov/patch

lib/core/httpclient4.js#L27-L28

Added lines #L27 - L28 were not covered by tests
}

function normalizeConfig(app) {
const config = app.config.httpclient;
if (typeof config.request.timeout === 'string') {
config.request.timeout = ms(config.request.timeout);
}

Check warning on line 35 in lib/core/httpclient4.js

View check run for this annotation

Codecov / codecov/patch

lib/core/httpclient4.js#L34-L35

Added lines #L34 - L35 were not covered by tests
}

module.exports = HttpClient4;
12 changes: 11 additions & 1 deletion lib/egg.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ const utils = require('./core/utils');
const BaseContextClass = require('./core/base_context_class');
const BaseHookClass = require('./core/base_hook_class');

let HttpClient4 = HttpClientNext;
const mainNodejsVersion = parseInt(process.versions.node.split('.')[0]);
if (mainNodejsVersion >= 18) {
// urllib@4 only works on Node.js >= 18
HttpClient4 = require('./core/httpclient4');
}

const HTTPCLIENT = Symbol('EggApplication#httpclient');
const LOGGERS = Symbol('EggApplication#loggers');
const EGG_PATH = Symbol.for('egg#eggPath');
Expand Down Expand Up @@ -51,6 +58,7 @@ class EggApplication extends EggCore {
this.ContextHttpClient = ContextHttpClient;
this.HttpClient = HttpClient;
this.HttpClientNext = HttpClientNext;
this.HttpClient4 = HttpClient4;

this.loader.loadConfig();

Expand Down Expand Up @@ -292,7 +300,9 @@ class EggApplication extends EggCore {
*/
createHttpClient(options) {
let httpClient;
if (this.config.httpclient.useHttpClientNext) {
if (this.config.httpclient.allowH2) {
httpClient = new this.HttpClient4(this);
} else if (this.config.httpclient.useHttpClientNext) {
httpClient = new this.HttpClientNext(this, options);
} else if (this.config.httpclient.enableDNSCache) {
httpClient = new DNSCacheHttpClient(this, options);
Expand Down
6 changes: 2 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@
"onelogger": "^1.0.0",
"sendmessage": "^2.0.0",
"urllib": "^2.33.0",
"urllib-next": "npm:urllib@^3.26.0",
"urllib-next": "npm:urllib@^3.27.1",
"urllib4": "npm:urllib@^4.3.0",
"utility": "^2.1.0",
"ylru": "^1.3.2"
},
Expand All @@ -77,19 +78,16 @@
"egg-view-nunjucks": "^2.3.0",
"eslint": "^8.23.1",
"eslint-config-egg": "^12.0.0",
"findlinks": "^2.2.0",
"formstream": "^1.1.1",
"jsdoc": "^3.6.11",
"koa": "^2.13.4",
"koa-static": "^5.0.0",
"node-libs-browser": "^2.2.1",
"pedding": "^1.1.0",
"prettier": "^2.7.1",
"puppeteer": "^19.11.1",
"react": "^16.14.0",
"react-dom": "^16.14.0",
"react-router": "^5.3.4",
"runscript": "^1.5.3",
"sdk-base": "^4.2.1",
"spy": "^1.0.0",
"supertest": "^6.2.4",
Expand Down
41 changes: 0 additions & 41 deletions test/doc.test.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
'use strict';

const assert = require('assert');

module.exports = app => {
class CustomHttpClient extends app.HttpClientNext {
class CustomHttpClient extends app.HttpClient4 {
request(url, opt) {
return new Promise(resolve => {
assert(/^http/.test(url), 'url should start with http, but got ' + url);
Expand All @@ -17,5 +15,5 @@ module.exports = app => {
return this.request(url, opt);
}
}
app.HttpClientNext = CustomHttpClient;
app.HttpClient4 = CustomHttpClient;
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
exports.httpclient = {
useHttpClientNext: true,
allowH2: true,
request: {
timeout: 99,
},
};
3 changes: 3 additions & 0 deletions test/fixtures/apps/httpclient-allowH2/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "httpclient-allowH2"
}
3 changes: 0 additions & 3 deletions test/fixtures/apps/httpclient-http2/package.json

This file was deleted.

4 changes: 2 additions & 2 deletions test/fixtures/apps/httpclient-tracer/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ module.exports = app => {
});
assert(res.status === 200);

res = await httpclient.request('https://github.com', {
res = await httpclient.request('https://registry.npmmirror.com', {
method: 'GET',
timeout: 20000,
});

assert(res.status === 200);

res = await httpclient.request('https://www.npmjs.com', {
res = await httpclient.request('https://npmmirror.com', {
method: 'GET',
timeout: 20000,
});
Expand Down
52 changes: 41 additions & 11 deletions test/lib/core/httpclient.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const assert = require('node:assert');
const { sensitiveHeaders } = require('node:http2');
const mm = require('egg-mock');
const urllib = require('urllib');
const Httpclient = require('../../../lib/core/httpclient');
Expand Down Expand Up @@ -305,7 +304,7 @@ describe('test/lib/core/httpclient.test.js', () => {
describe('overwrite httpclient support allowH2=true', () => {
let app;
before(() => {
app = utils.app('apps/httpclient-http2');
app = utils.app('apps/httpclient-allowH2');
return app.ready();
});
after(() => app.close());
Expand All @@ -314,21 +313,52 @@ describe('test/lib/core/httpclient.test.js', () => {
const res = await app.httpclient.request(url);
assert.equal(res.status, 200);
assert.equal(res.data.toString(), 'GET /');
assert.equal(sensitiveHeaders in res.headers, false);
// assert.equal(sensitiveHeaders in res.headers, false);
const res2 = await app.httpclient.request('https://registry.npmmirror.com/urllib/latest', {
dataType: 'json',
});
assert.equal(res2.status, 200);
assert.equal(res2.data.name, 'urllib');
assert.equal(sensitiveHeaders in res2.headers, true);
// assert.equal(sensitiveHeaders in res2.headers, true);
});

it('should assert url', () => {
return app.httpclient.curl('unknown url')
.catch(err => {
assert(err);
assert(err.message.includes('url should start with http, but got unknown url'));
it('should set request default global timeout to 99ms', async () => {
await assert.rejects(async () => {
await app.httpclient.curl(`${url}/timeout`);
}, err => {
assert.equal(err.name, 'HttpClientRequestTimeoutError');
assert(err.message.includes('Request timeout for 99 ms'));
return true;
});
});

it('should request http1.1 success', async () => {
const result = await app.httpclient.curl(`${url}`, {
dataType: 'text',
});
assert.equal(result.status, 200);
assert.equal(result.data, 'GET /');
});

it('should request http2 success', async () => {
for (let i = 0; i < 10; i++) {
const result = await app.httpclient.curl('https://registry.npmmirror.com', {
dataType: 'json',
timeout: 5000,
});
assert.equal(result.status, 200);
assert.equal(result.headers['content-type'], 'application/json; charset=utf-8');
assert.equal(result.data.sync_model, 'all');
}
});

it('should assert url', async () => {
await assert.rejects(async () => {
await app.httpclient.curl('unknown url');
}, err => {
assert.match(err.message, /url should start with http, but got unknown url/);
return true;
});
});
});

Expand Down Expand Up @@ -618,13 +648,13 @@ describe('test/lib/core/httpclient.test.js', () => {
});
assert(res.status === 200);

res = await httpclient.request('https://github.com', {
res = await httpclient.request('https://registry.npmmirror.com', {
method: 'GET',
timeout: 20000,
});
assert(res.status === 200);

res = await httpclient.request('https://www.npmjs.com', {
res = await httpclient.request('https://npmmirror.com', {
method: 'GET',
timeout: 20000,
});
Expand Down

0 comments on commit f9c1e89

Please sign in to comment.