fix(logger-plugin): log target port when router option is used (#1001)

* 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

CHANGELOG.md

@@ -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)
 

cspell.json

@@ -28,6 +28,7 @@
     "fastify",
     "globbing",
     "gzipped",
+    "jsonplaceholder",
     "lcov",
     "Lenna",
     "lipsum",
@@ -45,6 +46,7 @@
     "restream",
     "snyk",
     "streamify",
+    "typicode",
     "vhosted",
     "websockets",
     "xfwd"

src/plugins/default/logger-plugin.ts

@@ -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 */
@@ -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}]`;

src/utils/logger-plugin.ts

@@ -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];
+}

test/e2e/http-proxy-middleware.spec.ts

@@ -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', () => {
@@ -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({
@@ -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);
 

test/unit/utils/logger-plugin.spec.ts

@@ -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);
+  });
+});