Skip to content

Commit

Permalink
refactor: minor type improvements (#895)
Browse files Browse the repository at this point in the history
  • Loading branch information
chimurai authored Mar 14, 2023
1 parent f2a9fe4 commit 3e84b15
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 58 deletions.
6 changes: 3 additions & 3 deletions src/handlers/response-interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ export function responseInterceptor<
*/
function decompress<TReq extends http.IncomingMessage = http.IncomingMessage>(
proxyRes: TReq,
contentEncoding: string
): TReq {
let _proxyRes = proxyRes;
contentEncoding?: string
): TReq | zlib.Gunzip | zlib.Inflate | zlib.BrotliDecompress {
let _proxyRes: TReq | zlib.Gunzip | zlib.Inflate | zlib.BrotliDecompress = proxyRes;
let decompress;

switch (contentEncoding) {
Expand Down
14 changes: 9 additions & 5 deletions src/http-proxy-middleware.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type * as net from 'net';
import type * as http from 'http';
import type * as https from 'https';
import type { RequestHandler, Options, Filter } from './types';
Expand Down Expand Up @@ -38,7 +39,7 @@ export class HttpProxyMiddleware<TReq, TRes> {
}

// https://github.com/Microsoft/TypeScript/wiki/'this'-in-TypeScript#red-flags-for-this
public middleware: RequestHandler = async (req, res, next?) => {
public middleware: RequestHandler = (async (req, res, next?) => {
if (this.shouldProxy(this.proxyOptions.pathFilter, req)) {
try {
const activeProxyOptions = await this.prepareProxyRequest(req);
Expand Down Expand Up @@ -73,7 +74,7 @@ export class HttpProxyMiddleware<TReq, TRes> {
// use initial request to access the server object to subscribe to http upgrade event
this.catchUpgradeRequest(server);
}
};
}) as RequestHandler;

private registerPlugins(proxy: httpProxy<TReq, TRes>, options: Options<TReq, TRes>) {
const plugins = getPlugins<TReq, TRes>(options);
Expand All @@ -93,7 +94,7 @@ export class HttpProxyMiddleware<TReq, TRes> {
}
};

private handleUpgrade = async (req: http.IncomingMessage, socket, head) => {
private handleUpgrade = async (req: http.IncomingMessage, socket: net.Socket, head: Buffer) => {
if (this.shouldProxy(this.proxyOptions.pathFilter, req)) {
const activeProxyOptions = await this.prepareProxyRequest(req);
this.proxy.ws(req, socket, head, activeProxyOptions);
Expand All @@ -104,7 +105,10 @@ export class HttpProxyMiddleware<TReq, TRes> {
/**
* Determine whether request should be proxied.
*/
private shouldProxy = (pathFilter: Filter<TReq>, req: http.IncomingMessage): boolean => {
private shouldProxy = (
pathFilter: Filter<TReq> | undefined,
req: http.IncomingMessage
): boolean => {
return matchPathFilter(pathFilter, req.url, req);
};

Expand Down Expand Up @@ -138,7 +142,7 @@ export class HttpProxyMiddleware<TReq, TRes> {
};

// Modify option.target when router present.
private applyRouter = async (req: http.IncomingMessage, options) => {
private applyRouter = async (req: http.IncomingMessage, options: Options<TReq, TRes>) => {
let newTarget;

if (options.router) {
Expand Down
4 changes: 3 additions & 1 deletion src/legacy/options-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function legacyOptionsAdapter<TReq, TRes>(
legacyContext: Filter<TReq> | LegacyOptions<TReq, TRes>,
legacyOptions: LegacyOptions<TReq, TRes>
): Options<TReq, TRes> {
let options: LegacyOptions<TReq, TRes>;
let options: LegacyOptions<TReq, TRes> = {};
let logger: Logger;

// https://github.com/chimurai/http-proxy-middleware/pull/716
Expand Down Expand Up @@ -58,6 +58,8 @@ export function legacyOptionsAdapter<TReq, TRes>(
} else if (legacyContext && !legacyOptions) {
options = { ...(legacyContext as LegacyOptions<TReq, TRes>) };
logger = getLegacyLogger(options);
} else {
logger = getLegacyLogger({}) as never;
}

// map old event names to new event names
Expand Down
20 changes: 10 additions & 10 deletions src/path-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type * as http from 'http';

export function matchPathFilter<TReq = http.IncomingMessage>(
pathFilter: Filter<TReq> = '/',
uri: string,
uri: string | undefined,
req: http.IncomingMessage
): boolean {
// single path
Expand All @@ -34,8 +34,8 @@ export function matchPathFilter<TReq = http.IncomingMessage>(

// custom matching
if (typeof pathFilter === 'function') {
const pathname = getUrlPathName(uri);
return pathFilter(pathname, req as unknown as TReq);
const pathname = getUrlPathName(uri) as string;
return pathFilter(pathname, req as TReq);
}

throw new Error(ERRORS.ERR_CONTEXT_MATCHER_GENERIC);
Expand All @@ -46,18 +46,18 @@ export function matchPathFilter<TReq = http.IncomingMessage>(
* @param {String} uri 'http://example.org/api/b/c/d.html'
* @return {Boolean}
*/
function matchSingleStringPath(pathFilter: string, uri: string) {
function matchSingleStringPath(pathFilter: string, uri?: string) {
const pathname = getUrlPathName(uri);
return pathname.indexOf(pathFilter) === 0;
return pathname?.indexOf(pathFilter) === 0;
}

function matchSingleGlobPath(pattern: string | string[], uri: string) {
const pathname = getUrlPathName(uri);
function matchSingleGlobPath(pattern: string | string[], uri?: string) {
const pathname = getUrlPathName(uri) as string;
const matches = micromatch([pathname], pattern);
return matches && matches.length > 0;
}

function matchMultiGlobPath(patternList: string | string[], uri: string) {
function matchMultiGlobPath(patternList: string | string[], uri?: string) {
return matchSingleGlobPath(patternList, uri);
}

Expand All @@ -66,7 +66,7 @@ function matchMultiGlobPath(patternList: string | string[], uri: string) {
* @param {String} uri 'http://example.org/api/b/c/d.html'
* @return {Boolean}
*/
function matchMultiPath(pathFilterList: string[], uri: string) {
function matchMultiPath(pathFilterList: string[], uri?: string) {
let isMultiPath = false;

for (const context of pathFilterList) {
Expand All @@ -85,7 +85,7 @@ function matchMultiPath(pathFilterList: string[], uri: string) {
* @param {String} uri from req.url
* @return {String} RFC 3986 path
*/
function getUrlPathName(uri: string) {
function getUrlPathName(uri?: string) {
return uri && url.parse(uri).pathname;
}

Expand Down
8 changes: 5 additions & 3 deletions src/path-rewriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import { Debug } from './debug';

const debug = Debug.extend('path-rewriter');

type RewriteRule = { regex: RegExp; value: string };

/**
* Create rewrite function, to cache parsed rewrite rules.
*
* @param {Object} rewriteConfig
* @return {Function} Function to rewrite paths; This function should accept `path` (request.url) as parameter
*/
export function createPathRewriter(rewriteConfig) {
let rulesCache;
let rulesCache: RewriteRule[];

if (!isValidRewriteConfig(rewriteConfig)) {
return;
Expand Down Expand Up @@ -52,8 +54,8 @@ function isValidRewriteConfig(rewriteConfig) {
}
}

function parsePathRewriteRules(rewriteConfig) {
const rules = [];
function parsePathRewriteRules(rewriteConfig: Record<string, string>) {
const rules: RewriteRule[] = [];

if (isPlainObj(rewriteConfig)) {
for (const [key, value] of Object.entries(rewriteConfig)) {
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/default/error-response-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import { getStatusCode } from '../../status-code';
import { Plugin } from '../../types';
import type * as http from 'http';

export const errorResponsePlugin: Plugin = (proxyServer, options) => {
proxyServer.on('error', (err, req, res: http.ServerResponse, target?) => {
proxyServer.on('error', (err, req, res, target?) => {
// Re-throw error. Not recoverable since req & res are empty.
if (!req && !res) {
throw err; // "Error: Must provide a proper URL as target"
}

if (res.writeHead && !res.headersSent) {
if ('writeHead' in res && !res.headersSent) {
const statusCode = getStatusCode((err as unknown as any).code);
res.writeHead(statusCode);
}
Expand Down
4 changes: 2 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface RequestHandler<
TNext = NextFunction
> {
(req: TReq, res: TRes, next?: TNext): void | Promise<void>;
upgrade?: (req: http.IncomingMessage, socket: net.Socket, head: any) => void;
upgrade: (req: http.IncomingMessage, socket: net.Socket, head: Buffer) => void;
}

export type Filter<TReq = http.IncomingMessage> =
Expand Down Expand Up @@ -67,7 +67,7 @@ export interface Options<TReq = http.IncomingMessage, TRes = http.ServerResponse
*/
pathRewrite?:
| { [regexp: string]: string }
| ((path: string, req: TReq) => string)
| ((path: string, req: TReq) => string | undefined)
| ((path: string, req: TReq) => Promise<string>);
/**
* Access the internal http-proxy server instance to customize behavior
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/express-error-middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('express error middleware', () => {
const app = createApp(proxyMiddleware, errorMiddleware);
const response = await request(app).get('/get').expect(504);

expect(httpProxyError.message).toBe('Must provide a proper URL as target');
expect(httpProxyError?.message).toBe('Must provide a proper URL as target');
expect(response.text).toBe('Something broke!');
});
});
8 changes: 4 additions & 4 deletions test/e2e/http-proxy-middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ describe('E2E http-proxy-middleware', () => {
});

it('should send request header "host" to target server', async () => {
let completedRequest: CompletedRequest;
let completedRequest: CompletedRequest | undefined;

await mockTargetServer.forGet().thenCallback((req) => {
completedRequest = req;
Expand All @@ -265,7 +265,7 @@ describe('E2E http-proxy-middleware', () => {

const response = await agent.get(`/api/some/endpoint/index.html`).expect(200);
expect(response.text).toBe('OK');
expect(completedRequest.headers.host).toBe('foobar.dev');
expect(completedRequest?.headers.host).toBe('foobar.dev');
});
});

Expand Down Expand Up @@ -374,15 +374,15 @@ describe('E2E http-proxy-middleware', () => {
});

it('should add `x-added` as custom header to request"', async () => {
let completedRequest: CompletedRequest;
let completedRequest: CompletedRequest | undefined;
await mockTargetServer.forGet().thenCallback((req) => {
completedRequest = req;
return { statusCode: 200 };
});

await agent.get(`/api/foo/bar`).expect(200);

expect(completedRequest.headers['x-added']).toBe('added-from-hpm');
expect(completedRequest?.headers['x-added']).toBe('added-from-hpm');
});
});

Expand Down
4 changes: 2 additions & 2 deletions test/e2e/plugins.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ describe('E2E Plugins', () => {
});

it('should register a plugin and access the http-proxy object', async () => {
let proxyReqUrl: string;
let responseStatusCode: number;
let proxyReqUrl: string | undefined;
let responseStatusCode: number | undefined;

mockTargetServer.forGet('/users/1').thenReply(200, '{"userName":"John"}');

Expand Down
41 changes: 18 additions & 23 deletions test/unit/response-interceptor.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { IncomingMessage, ServerResponse } from 'http';
import { Socket } from 'net';

import { responseInterceptor } from '../../src/handlers/response-interceptor';

const fakeProxyResponse = () => {
const httpIncomingMessage = new IncomingMessage(null);
const httpIncomingMessage = new IncomingMessage(new Socket());
httpIncomingMessage._read = () => ({});
return httpIncomingMessage;
};
Expand All @@ -24,38 +25,32 @@ const waitInterceptorHandler = (ms = 1): Promise<void> =>

describe('responseInterceptor', () => {
it('should write body on end proxy event', async () => {
const httpIncomingMessage = fakeProxyResponse();
const response = fakeResponse();
const proxyRes = fakeProxyResponse();
const req = fakeProxyResponse();
const res = fakeResponse();

responseInterceptor(async () => JSON.stringify({ someField: '' }))(
httpIncomingMessage,
null,
response
);
responseInterceptor(async () => JSON.stringify({ someField: '' }))(proxyRes, req, res);

httpIncomingMessage.emit('end');
proxyRes.emit('end');
await waitInterceptorHandler();

const expectedBody = JSON.stringify({ someField: '' });
expect(response.setHeader).toHaveBeenCalledWith('content-length', expectedBody.length);
expect(response.write).toHaveBeenCalledWith(Buffer.from(expectedBody));
expect(response.end).toHaveBeenCalledWith();
expect(res.setHeader).toHaveBeenCalledWith('content-length', expectedBody.length);
expect(res.write).toHaveBeenCalledWith(Buffer.from(expectedBody));
expect(res.end).toHaveBeenCalledWith();
});

it('should end with error when receive a proxy error event', async () => {
const httpIncomingMessage = fakeProxyResponse();
const response = fakeResponse();
const proxyRes = fakeProxyResponse();
const req = fakeProxyResponse();
const res = fakeResponse();

responseInterceptor(async () => JSON.stringify({ someField: '' }))(
httpIncomingMessage,
null,
response
);
responseInterceptor(async () => JSON.stringify({ someField: '' }))(proxyRes, req, res);

httpIncomingMessage.emit('error', new Error('some error message'));
proxyRes.emit('error', new Error('some error message'));

expect(response.setHeader).not.toHaveBeenCalled();
expect(response.write).not.toHaveBeenCalled();
expect(response.end).toHaveBeenCalledWith('Error fetching proxied request: some error message');
expect(res.setHeader).not.toHaveBeenCalled();
expect(res.write).not.toHaveBeenCalled();
expect(res.end).toHaveBeenCalledWith('Error fetching proxied request: some error message');
});
});
4 changes: 3 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
"moduleResolution": "node",
"target": "es2019",
"incremental": true,
"declaration": true
"declaration": true,
"strict": true,
"noImplicitAny": false
},
"include": ["./src"]
}

0 comments on commit 3e84b15

Please sign in to comment.