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

fix: add createHttpClient back to app instance #5383

Merged
merged 2 commits into from
Jan 14, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/app/extend/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import type Request from './request.js';
import type Response from './response.js';
import type Helper from './helper.js';

import './context.types.js';

const HELPER = Symbol('ctx helper');
const LOCALS = Symbol('ctx locals');
const LOCALS_LIST = Symbol('ctx localsList');
Expand Down Expand Up @@ -306,3 +304,19 @@ export default class Context extends EggCoreContext {
this.response.realStatus = val;
}
}

declare module '@eggjs/core' {
// add Context overrides types
interface Context {
curl(url: HttpClientRequestURL, options?: HttpClientRequestOptions): ReturnType<HttpClient['request']>;
get router(): Router;
set router(val: Router);
get helper(): Helper;
get httpclient(): HttpClient;
get httpClient(): HttpClient;
getLogger(name: string): EggLogger;
get logger(): EggLogger;
get coreLogger(): EggLogger;
get locals(): Record<string, any>;
}
}
24 changes: 0 additions & 24 deletions src/app/extend/context.types.ts

This file was deleted.

13 changes: 11 additions & 2 deletions src/app/extend/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ const HOST = Symbol('request host');
const IPS = Symbol('request ips');
const RE_ARRAY_KEY = /[^\[\]]+\[\]$/;

import './request.types.js';

export default class Request extends EggCoreRequest {
declare app: Application;
declare ctx: Context;
Expand Down Expand Up @@ -266,6 +264,17 @@ export default class Request extends EggCoreRequest {
}
}

declare module '@eggjs/core' {
// add Request overrides types
interface Request {
body: any;
get acceptJSON(): boolean;
get query(): Record<string, string>;
set query(obj: Record<string, string>);
get queries(): Record<string, string[]>;
}
}

function firstValue(value: string | string[]) {
if (Array.isArray(value)) {
value = value[0];
Expand Down
10 changes: 0 additions & 10 deletions src/app/extend/request.types.ts

This file was deleted.

10 changes: 8 additions & 2 deletions src/app/extend/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { Response as KoaResponse } from '@eggjs/core';

const REAL_STATUS = Symbol('response realStatus');

import './response.types.js';

export default class Response extends KoaResponse {
/**
* Get or set a real status code.
Expand Down Expand Up @@ -36,3 +34,11 @@ export default class Response extends KoaResponse {
this[REAL_STATUS] = status;
}
}

declare module '@eggjs/core' {
// add Response overrides types
interface Response {
get realStatus(): number;
set realStatus(status: number);
}
}
7 changes: 0 additions & 7 deletions src/app/extend/response.types.ts

This file was deleted.

15 changes: 11 additions & 4 deletions src/lib/core/httpclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import {
HttpClient as RawHttpClient,
RequestURL as HttpClientRequestURL,
RequestOptions,
ClientOptions as HttpClientOptions,
} from 'urllib';
import { ms } from 'humanize-ms';
import type { EggApplicationCore } from '../egg.js';

export type {
HttpClientResponse,
RequestURL as HttpClientRequestURL,
ClientOptions as HttpClientOptions,
} from 'urllib';

export interface HttpClientRequestOptions extends RequestOptions {
Expand All @@ -19,12 +21,17 @@ export interface HttpClientRequestOptions extends RequestOptions {
export class HttpClient extends RawHttpClient {
readonly #app: EggApplicationCore & { tracer?: any };

constructor(app: EggApplicationCore) {
constructor(app: EggApplicationCore, options: HttpClientOptions = {}) {
normalizeConfig(app);
const config = app.config.httpclient;
super({
defaultArgs: config.request,
});
const initOptions: HttpClientOptions = {
...options,
defaultArgs: {
...config.request,
...options.defaultArgs,
},
};
super(initOptions);
this.#app = app;
}

Expand Down
34 changes: 29 additions & 5 deletions src/lib/egg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import type { EggAppConfig } from './types.js';
import { create as createMessenger, IMessenger } from './core/messenger/index.js';
import { ContextHttpClient } from './core/context_httpclient.js';
import {
HttpClient, type HttpClientRequestOptions, type HttpClientRequestURL, type HttpClientResponse,
HttpClient,
type HttpClientRequestOptions, type HttpClientRequestURL, type HttpClientResponse,
type HttpClientOptions,
} from './core/httpclient.js';
import { createLoggers } from './core/logger.js';
import {
Expand All @@ -43,8 +45,6 @@ import { BaseHookClass } from './core/base_hook_class.js';
import type { EggApplicationLoader } from './loader/index.js';
import { getSourceDirname } from './utils.js';

import './egg.types.js';

const EGG_PATH = Symbol.for('egg#eggPath');

export interface EggApplicationCoreOptions extends Omit<EggCoreOptions, 'baseDir'> {
Expand Down Expand Up @@ -387,14 +387,22 @@ export class EggApplicationCore extends EggCore {
return await this.httpClient.request<T>(url, options);
}

/**
* Create a new HttpClient instance with custom options
* @param {Object} [options] HttpClient init options
*/
createHttpClient(options?: HttpClientOptions): HttpClient {
return new this.HttpClient(this, options);
}

/**
* HttpClient instance
* @see https://github.com/node-modules/urllib
* @member {HttpClient}
*/
get httpClient() {
get httpClient(): HttpClient {
if (!this.#httpClient) {
this.#httpClient = new this.HttpClient(this);
this.#httpClient = this.createHttpClient();
}
return this.#httpClient;
}
Expand Down Expand Up @@ -680,3 +688,19 @@ export class EggApplicationCore extends EggCore {
return context;
}
}

declare module '@eggjs/core' {
// add EggApplicationCore overrides types
interface EggCore {
inspect(): any;
get currentContext(): EggContext | undefined;
ctxStorage: AsyncLocalStorage<EggContext>;
get logger(): EggLogger;
get coreLogger(): EggLogger;
getLogger(name: string): EggLogger;
createHttpClient(options?: HttpClientOptions): HttpClient;
HttpClient: typeof HttpClient;
get httpClient(): HttpClient;
curl<T = any>(url: HttpClientRequestURL, options?: HttpClientRequestOptions): Promise<HttpClientResponse<T>>;
}
}
15 changes: 0 additions & 15 deletions src/lib/egg.types.ts

This file was deleted.

53 changes: 12 additions & 41 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ export interface CustomLoaderConfig extends Omit<FileLoaderOptions, 'inject' | '
loadunit?: boolean;
}

export interface HttpClientConfig {
/** Request timeout */
timeout?: number;
/** Default request args for httpclient */
request?: HttpClientRequestOptions;
/**
* @deprecated keep compatible with egg 3.x, no more used
*/
useHttpClientNext?: boolean;
}

export interface EggAppConfig extends EggCoreAppConfig {
workerStartTimeout: number;
baseDir: string;
Expand Down Expand Up @@ -141,47 +152,7 @@ export interface EggAppConfig extends EggCoreAppConfig {
};

/** Configuration of httpclient in egg. */
httpclient: {
/** Request timeout */
timeout?: number;
/** Default request args for httpclient */
request?: HttpClientRequestOptions;
/**
* @deprecated keep compatible with egg 3.x, no more used
*/
useHttpClientNext?: boolean;
};

// development: {
// /**
// * dirs needed watch, when files under these change, application will reload, use relative path
// */
// watchDirs: string[];
// /**
// * dirs don't need watch, including subdirectories, use relative path
// */
// ignoreDirs: string[];
// /**
// * don't wait all plugins ready, default is true.
// */
// fastReady: boolean;
// /**
// * whether reload on debug, default is true.
// */
// reloadOnDebug: boolean;
// /**
// * whether override default watchDirs, default is false.
// */
// overrideDefault: boolean;
// /**
// * whether override default ignoreDirs, default is false.
// */
// overrideIgnore: boolean;
// /**
// * whether to reload, use https://github.com/sindresorhus/multimatch
// */
// reloadPattern: string[] | string;
// };
httpclient: HttpClientConfig;

/**
* customLoader config
Expand Down
19 changes: 19 additions & 0 deletions test/lib/core/httpclient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,4 +634,23 @@ describe('test/lib/core/httpclient.test.ts', () => {
assert(res.status === 200);
});
});

describe('app.createHttpClient(options)', () => {
let app: MockApplication;
before(() => {
app = createApp('apps/httpclient-retry');
return app.ready();
});
after(() => app.close());

it('should work', async () => {
const client1 = app.createHttpClient();
const client2 = app.createHttpClient();
assert.notEqual(client1, client2);
const res = await client1.request(url, {
method: 'GET',
});
assert.equal(res.status, 200);
});
});
Comment on lines +638 to +655
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance test coverage for createHttpClient.

While the current tests verify basic functionality, consider adding tests for:

  1. Custom options handling
  2. Error scenarios
  3. SSRF protection functionality (main PR objective)

Example test cases:

it('should apply custom options', () => {
  const client = app.createHttpClient({ timeout: 1000 });
  assert.equal(client.options.timeout, 1000);
});

it('should handle SSRF protection', async () => {
  const client = app.createHttpClient();
  await assert.rejects(
    () => client.request('http://internal-network'),
    /SSRF protection/
  );
});

});
Loading
Loading