Address code review issues

Author: vhess

README.md

@@ -452,7 +452,7 @@ http-proxy-3 supports HTTP/2 through the native fetch API. When fetch is enabled
 
 ```js
 import { createProxyServer } from "http-proxy-3";
-import { Agent } from "undici";
+import { Agent, setGlobalDispatcher } from "undici";
 
 // Either enable HTTP/2 for all fetch operations
 setGlobalDispatcher(new Agent({ allowH2: true }));

biome.json

@@ -0,0 +1,35 @@
+{
+	"$schema": "https://biomejs.dev/schemas/2.2.5/schema.json",
+	"vcs": {
+		"enabled": false,
+		"clientKind": "git",
+		"useIgnoreFile": false
+	},
+	"files": {
+		"ignoreUnknown": false
+	},
+	"formatter": {
+		"enabled": true,
+		"indentStyle": "space",
+		"lineWidth": 120
+	},
+	"linter": {
+		"enabled": true,
+		"rules": {
+			"recommended": true
+		}
+	},
+	"javascript": {
+		"formatter": {
+			"quoteStyle": "double"
+		}
+	},
+	"assist": {
+		"enabled": true,
+		"actions": {
+			"source": {
+				"organizeImports": "on"
+			}
+		}
+	}
+}

lib/http-proxy/passes/web-incoming.ts

@@ -7,10 +7,7 @@ The names of passes are exported as WEB_PASSES from this module.
 
 */
 
-import type {
-  IncomingMessage as Request,
-  ServerResponse as Response,
-} from "node:http";
+import type { IncomingMessage as Request, ServerResponse as Response } from "node:http";
 import * as http from "node:http";
 import * as https from "node:https";
 import type { Socket } from "node:net";
@@ -41,10 +38,7 @@ const nativeAgents = { http, https };
 
 //  Sets `content-length` to '0' if request is of DELETE type.
 export function deleteLength(req: Request) {
-  if (
-    (req.method === "DELETE" || req.method === "OPTIONS") &&
-    !req.headers["content-length"]
-  ) {
+  if ((req.method === "DELETE" || req.method === "OPTIONS") && !req.headers["content-length"]) {
     req.headers["content-length"] = "0";
     delete req.headers["transfer-encoding"];
   }
@@ -72,13 +66,10 @@ export function XHeaders(req: Request, _res: Response, options: ServerOptions) {
 
   for (const header of ["for", "port", "proto"] as const) {
     req.headers["x-forwarded-" + header] =
-      (req.headers["x-forwarded-" + header] || "") +
-      (req.headers["x-forwarded-" + header] ? "," : "") +
-      values[header];
+      (req.headers["x-forwarded-" + header] || "") + (req.headers["x-forwarded-" + header] ? "," : "") + values[header];
   }
 
-  req.headers["x-forwarded-host"] =
-    req.headers["x-forwarded-host"] || req.headers["host"] || "";
+  req.headers["x-forwarded-host"] = req.headers["x-forwarded-host"] || req.headers["host"] || "";
 }
 
 // Does the actual proxying. If `forward` is enabled fires up
@@ -106,12 +97,7 @@ export function stream(
   if (options.forward) {
     // forward enabled, so just pipe the request
     const proto = options.forward.protocol === "https:" ? https : http;
-    const outgoingOptions = common.setupOutgoing(
-      options.ssl || {},
-      options,
-      req,
-      "forward",
-    );
+    const outgoingOptions = common.setupOutgoing(options.ssl || {}, options, req, "forward");
     const forwardReq = proto.request(outgoingOptions);
 
     // error handler (e.g. ECONNRESET, ECONNREFUSED)
@@ -162,15 +148,9 @@ export function stream(
   req.on("error", proxyError);
   proxyReq.on("error", proxyError);
 
-  function createErrorHandler(
-    proxyReq: http.ClientRequest,
-    url: NormalizeProxyTarget<ProxyTargetUrl>,
-  ) {
+  function createErrorHandler(proxyReq: http.ClientRequest, url: NormalizeProxyTarget<ProxyTargetUrl>) {
     return (err: Error) => {
-      if (
-        req.socket.destroyed &&
-        (err as NodeJS.ErrnoException).code === "ECONNRESET"
-      ) {
+      if (req.socket.destroyed && (err as NodeJS.ErrnoException).code === "ECONNRESET") {
         server.emit("econnreset", err, req, res, url);
         proxyReq.destroy();
         return;
@@ -236,10 +216,7 @@ async function stream2(
   };
 
   req.on("error", (err: Error) => {
-    if (
-      req.socket.destroyed &&
-      (err as NodeJS.ErrnoException).code === "ECONNRESET"
-    ) {
+    if (req.socket.destroyed && (err as NodeJS.ErrnoException).code === "ECONNRESET") {
       const target = options.target || options.forward;
       if (target) {
         server.emit("econnreset", err, req, res, target);
@@ -249,19 +226,13 @@ async function stream2(
     handleError(err);
   });
 
-  const fetchOptions =
-    options.fetch === true ? ({} as FetchOptions) : options.fetch;
+  const fetchOptions = options.fetch === true ? ({} as FetchOptions) : options.fetch;
   if (!fetchOptions) {
     throw new Error("stream2 called without fetch options");
   }
 
   if (options.forward) {
-    const outgoingOptions = common.setupOutgoing(
-      options.ssl || {},
-      options,
-      req,
-      "forward",
-    );
+    const outgoingOptions = common.setupOutgoing(options.ssl || {}, options, req, "forward");
 
     const requestOptions: RequestInit = {
       method: outgoingOptions.method,
@@ -290,10 +261,7 @@ async function stream2(
     }
 
     try {
-      const result = await fetch(
-        new URL(outgoingOptions.url).origin + outgoingOptions.path,
-        requestOptions,
-      );
+      const result = await fetch(new URL(outgoingOptions.url).origin + outgoingOptions.path, requestOptions);
 
       // Call onAfterResponse callback for forward requests (though they typically don't expect responses)
       if (fetchOptions.onAfterResponse) {
@@ -341,6 +309,7 @@ async function stream2(
     requestOptions.body = options.buffer as Stream.Readable;
   } else if (req.method !== "GET" && req.method !== "HEAD") {
     requestOptions.body = req;
+    requestOptions.duplex = "half";
   }
 
   // Call onBeforeRequest callback before making the request
@@ -354,10 +323,7 @@ async function stream2(
   }
 
   try {
-    const response = await fetch(
-      new URL(outgoingOptions.url).origin + outgoingOptions.path,
-      requestOptions,
-    );
+    const response = await fetch(new URL(outgoingOptions.url).origin + outgoingOptions.path, requestOptions);
 
     // Call onAfterResponse callback after receiving the response
     if (fetchOptions.onAfterResponse) {
@@ -402,9 +368,7 @@ async function stream2(
 
     if (!res.writableEnded) {
       // Allow us to listen for when the proxy has completed
-      const nodeStream = response.body
-        ? Readable.from(response.body as AsyncIterable<Uint8Array>)
-        : null;
+      const nodeStream = response.body ? Readable.from(response.body as AsyncIterable<Uint8Array>) : null;
 
       if (nodeStream) {
         nodeStream.on("error", (err) => {
@@ -428,9 +392,7 @@ async function stream2(
       server?.emit("end", req, res, fakeProxyRes);
     }
   } catch (err) {
-    if (err) {
-      handleError(err as Error, options.target);
-    }
+    handleError(err as Error, options.target);
   }
 }
 

lib/http-proxy/passes/web-outgoing.ts

@@ -143,7 +143,7 @@ export function writeHeaders(
 
   for (const key0 in proxyRes.headers) {
     let key = key0;
-    if (_req.httpVersionMajor > 1 && (key === "connection") || key === "keep-alive") {
+    if (_req.httpVersionMajor > 1 && (key === "connection" || key === "keep-alive")) {  
       // don't send connection header to http2 client
       continue;
     }

lib/test/http/proxy-callbacks.test.ts

@@ -64,7 +64,8 @@ describe("Fetch callback functions (onBeforeRequest and onAfterResponse)", () =>
                     console.log(`Response received: ${response.status}`);
                 }
             }
-        }); servers.proxy = proxy.listen(ports.proxy);
+        }); 
+        servers.proxy = proxy.listen(ports.proxy);
 
         // Make a request through the proxy
         const response = await fetch(`http://localhost:${ports.proxy}/test`);

lib/test/http/proxy-http2-to-http.test.ts

@@ -7,19 +7,15 @@ import * as httpProxy from "../..";
 import getPort from "../get-port";
 import { join } from "node:path";
 import { readFile } from "node:fs/promises";
-import fetch from "node-fetch";
-import { describe, it, expect, beforeAll, afterAll } from 'vitest';
-import { Agent, setGlobalDispatcher } from "undici";
-
-setGlobalDispatcher(new Agent({
-  allowH2: true
-}));
+import { describe, it, expect, beforeAll, afterAll } from "vitest";
+import { Agent, fetch } from "undici";
 
+const TestAgent = new Agent({ allowH2: true });
 
 const fixturesDir = join(__dirname, "..", "fixtures");
 
 describe("Basic example of proxying over HTTPS to a target HTTP server", () => {
-  let ports: Record<'http' | 'proxy', number>;
+  let ports: Record<"http" | "proxy", number>;
   beforeAll(async () => {
     ports = { http: await getPort(), proxy: await getPort() };
   });
@@ -52,12 +48,12 @@ describe("Basic example of proxying over HTTPS to a target HTTP server", () => {
   });
 
   it("Use fetch to test non-https server", async () => {
-    const r = await (await fetch(`http://localhost:${ports.http}`)).text();
+    const r = await (await fetch(`http://localhost:${ports.http}`, { dispatcher: TestAgent })).text();
     expect(r).toContain("hello http over https");
   });
 
   it("Use fetch to test the ACTUAL https server", async () => {
-    const r = await (await fetch(`https://localhost:${ports.proxy}`)).text();
+    const r = await (await fetch(`https://localhost:${ports.proxy}`, { dispatcher: TestAgent })).text();
     expect(r).toContain("hello http over https");
   });
 

lib/test/http/proxy-http2-to-http2.test.ts

@@ -1,5 +1,5 @@
 /*
-pnpm test proxy-https-to-https.test.ts
+pnpm test proxy-http2-to-http2.test.ts
 
 */
 
@@ -8,17 +8,15 @@ import * as httpProxy from "../..";
 import getPort from "../get-port";
 import { join } from "node:path";
 import { readFile } from "node:fs/promises";
-import { describe, it, expect, beforeAll, afterAll } from 'vitest';
-import { Agent, setGlobalDispatcher } from "undici";
+import { describe, it, expect, beforeAll, afterAll } from "vitest";
+import { Agent, fetch } from "undici";
 
-setGlobalDispatcher(new Agent({
-  allowH2: true
-}));
+const TestAgent = new Agent({ allowH2: true, connect: { rejectUnauthorized: false } });
 
 const fixturesDir = join(__dirname, "..", "fixtures");
 
 describe("Basic example of proxying over HTTP2 to a target HTTP2 server", () => {
-  let ports: Record<'http2' | 'proxy', number>;
+  let ports: Record<"http2" | "proxy", number>;
   beforeAll(async () => {
     // Gets ports
     ports = { http2: await getPort(), proxy: await getPort() };
@@ -46,20 +44,20 @@ describe("Basic example of proxying over HTTP2 to a target HTTP2 server", () =>
       .createServer({
         target: `https://localhost:${ports.http2}`,
         ssl,
-        fetch: { dispatcher: new Agent({ allowH2: true }) as any },
+        fetch: { dispatcher: TestAgent as any },
         // without secure false, clients will fail and this is broken:
         secure: false,
       })
       .listen(ports.proxy);
   });
 
   it("Use fetch to test direct non-proxied http2 server", async () => {
-    const r = await (await fetch(`https://localhost:${ports.http2}`)).text();
+    const r = await (await fetch(`https://localhost:${ports.http2}`, { dispatcher: TestAgent })).text();
     expect(r).toContain("hello over http2");
   });
 
   it("Use fetch to test the proxy server", async () => {
-    const r = await (await fetch(`https://localhost:${ports.proxy}`)).text();
+    const r = await (await fetch(`https://localhost:${ports.proxy}`, { dispatcher: TestAgent })).text();
     expect(r).toContain("hello over http2");
   });
 

lib/test/http/proxy-http2-to-https.test.ts

@@ -8,17 +8,12 @@ import * as httpProxy from "../..";
 import getPort from "../get-port";
 import { join } from "node:path";
 import { readFile } from "node:fs/promises";
-import { describe, it, expect, beforeAll, afterAll } from 'vitest';
-import { Agent, setGlobalDispatcher } from "undici";
-
-setGlobalDispatcher(new Agent({
-  allowH2: true
-}));
+import { describe, it, expect, beforeAll, afterAll } from "vitest";
 
 const fixturesDir = join(__dirname, "..", "fixtures");
 
 describe("Basic example of proxying over HTTPS to a target HTTPS server", () => {
-  let ports: Record<'https' | 'proxy', number>;
+  let ports: Record<"https" | "proxy", number>;
   beforeAll(async () => {
     // Gets ports
     ports = { https: await getPort(), proxy: await getPort() };