Skip to content

Commit

Permalink
fix(logger-plugin): log target port when router option is used (#1001)
Browse files Browse the repository at this point in the history
* fix(logger-plugin): log target port when router option is used
* refactor(logger-plugin): extract getPort logic and add some tests
* ci(spellcheck): add jsonplaceholder.typicode.com
  • Loading branch information
chimurai authored May 10, 2024
1 parent 4d3f549 commit f2a0af3
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- refactor(errors): improve pathFilter error message
- fix(logger-plugin): fix missing target port
- ci(package): npm package provenance
- fix(logger-plugin): log target port when router option is used

## [v3.0.0](https://github.com/chimurai/http-proxy-middleware/releases/tag/v3.0.0)

Expand Down
2 changes: 2 additions & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"fastify",
"globbing",
"gzipped",
"jsonplaceholder",
"lcov",
"Lenna",
"lipsum",
Expand All @@ -45,6 +46,7 @@
"restream",
"snyk",
"streamify",
"typicode",
"vhosted",
"websockets",
"xfwd"
Expand Down
16 changes: 14 additions & 2 deletions src/plugins/default/logger-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { URL } from 'url';
import { Plugin } from '../../types';
import { getLogger } from '../../logger';
import type { IncomingMessage } from 'node:http';
import { getPort } from '../../utils/logger-plugin';

type ExpressRequest = {
/** Express req.baseUrl */
Expand Down Expand Up @@ -43,8 +44,19 @@ export const loggerPlugin: Plugin = (proxyServer, options) => {
const originalUrl = req.originalUrl ?? `${req.baseUrl || ''}${req.url}`;

// construct targetUrl
const target = new URL(options.target as URL);
target.pathname = proxyRes.req.path;
const port = getPort(proxyRes.req?.agent?.sockets);

const obj = {
protocol: proxyRes.req.protocol,
host: proxyRes.req.host,
pathname: proxyRes.req.path,
} as URL;

const target = new URL(`${obj.protocol}//${obj.host}${obj.pathname}`);

if (port) {
target.port = port;
}
const targetUrl = target.toString();

const exchange = `[HPM] ${req.method} ${originalUrl} -> ${targetUrl} [${proxyRes.statusCode}]`;
Expand Down
11 changes: 11 additions & 0 deletions src/utils/logger-plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import type { Agent } from 'node:http';

export type Sockets = Pick<Agent, 'sockets'>;

/**
* Get port from target
* Using proxyRes.req.agent.sockets to determine the target port
*/
export function getPort(sockets?: Sockets): string | undefined {
return Object.keys(sockets || {})?.[0]?.split(':')[1];
}
27 changes: 25 additions & 2 deletions test/e2e/http-proxy-middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Mockttp, getLocal, CompletedRequest } from 'mockttp';
import type * as http from 'http';
import type * as express from 'express';
import * as bodyParser from 'body-parser';
import type { Logger } from '../../src/types';

describe('E2E http-proxy-middleware', () => {
describe('http-proxy-middleware creation', () => {
Expand Down Expand Up @@ -433,15 +434,18 @@ describe('E2E http-proxy-middleware', () => {

describe('option.logger', () => {
let logMessages: string[];
let customLogger: Logger;

beforeEach(() => {
logMessages = [];
const customLogger = {
customLogger = {
info: (message: string) => logMessages.push(message),
warn: (message: string) => logMessages.push(message),
error: (message: string) => logMessages.push(message),
};
});

it('should have logged messages', async () => {
agent = request(
createApp(
createProxyMiddleware({
Expand All @@ -451,9 +455,28 @@ describe('E2E http-proxy-middleware', () => {
}),
),
);

await mockTargetServer.forGet('/api/foo/bar').thenReply(200);
await agent.get(`/api/foo/bar`).expect(200);

expect(logMessages).not.toBeUndefined();
expect(logMessages.length).toBe(1);
expect(logMessages.at(0)).toBe(
`[HPM] GET /api/foo/bar -> http://localhost:${mockTargetServer.port}/api/foo/bar [200]`,
);
});

it('should have logged messages', async () => {
it('should have logged messages when router used', async () => {
agent = request(
createApp(
createProxyMiddleware({
router: () => `http://localhost:${mockTargetServer.port}`,
pathFilter: '/api',
logger: customLogger,
}),
),
);

await mockTargetServer.forGet('/api/foo/bar').thenReply(200);
await agent.get(`/api/foo/bar`).expect(200);

Expand Down
23 changes: 23 additions & 0 deletions test/unit/utils/logger-plugin.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { getPort, type Sockets } from '../../../src/utils/logger-plugin';

describe('getPort()', () => {
it('should return port from proxyRes.req.agent.sockets', () => {
const sockets = {
'jsonplaceholder.typicode.com:80:': [],
} as unknown as Sockets;

expect(getPort(sockets)).toBe('80');
});

it('should handle missing "sockets" from proxyRes?.req?.agent?.sockets', () => {
const sockets = undefined;

expect(getPort(sockets)).toBe(undefined);
});

it('should handle empty "sockets" from proxyRes?.req?.agent?.sockets', () => {
const sockets = {} as unknown as Sockets;

expect(getPort(sockets)).toBe(undefined);
});
});

0 comments on commit f2a0af3

Please sign in to comment.