POC: Request through undici

[Corvince]

This is a proof of concept to explore HTTP/2 proxy support options. The ultimate goal is enabling full HTTP/2 proxy support, which presents a significant challenge: we currently proxy through HTTP/HTTPS requests, but can't detect HTTP/2 support beforehand. This would require attempting HTTP/2 first (using a different API), then falling back to regular requests - adding considerable complexity.

I explored using the Undici client instead, which offers:

• Unified API for HTTP/1.1, HTTPS, and HTTP/2
• Built-in features (connection pooling, redirects, etc.)
• Performance improvements
• Automatic protocol negotiation

To my surprise the code already passes most of the http and websocket tests (but struggling on lib tests).

This approach has significant API compatibility issues:

1. No custom agent support - HTTP/HTTPS agents are no longer used, breaking existing customizations.

2. Event model changes - The proxyReq event no longer exists, though similar functionality could be provided via interceptors. This is the most critical issue since many integrations depend on these events.

The latter point is where this approach falls together, because lots of things are built around these events. But the same would be somewhat true if we use a http2 client request, there is no API compatible way to handle both http2 requests.

So I am somewhat unsure how to proceed from here. First of all as I understand it http-proxy-3 should be a drop-in replacement for http-proxy, so any API changes should probably be forbidden at this point (especially given the rather low adaptation level currently). This leaves some option to consider

1. Just leave it as it is (supporting only incoming http2 requests), which probably isn't super future-proof
2. Create a new API for http2 requests (which then could also be used for http1/http1.1 used)
3. Try to recreate and/or emulate the proxyReq/proxyRes objects for http2 requests, which probably only works for simple things (e.g. returning an object with a setHeader method).

So I think the path forward depends on the general path forward and current use cases for this library, which I cannot evaluate.

[williamstein]

It seems to me that the best option is "Create a new API for http2 requests (which then could also be used for http1/http1.1 used)". This keeps things backward compatible -- which is a hard requirement for this project -- but still provides a path for http2 support.

[Corvince]

Probably makes sense. I started to go in this direction by adding "clientOptions" and "requestOptions", which are undici specific. If either of those are set, the new code path will trigger. The next step will be assessing the compatibility of the existing options with this new code path. Then I will be able to figure out when and how to warn for incompatible options . For example, setting dispatchOptions and a custom http agent will never be compatible, while followRedirects can, but simply has not been implemented so far.

[Corvince]

Staring at this stuff for a few more hours I realized that most of the options stuff happens outside of the "stream" function anyway. With the current modifications almost all tests pass, the exceptions being only

1. Tests depending on proxyReq event (1 direct tests one middleware test)
2. Tests depending on proxyRes event (2 tests)
3. A test that checks CamelCase of some headers (they are always lowercased with undici)
4. A test that checks if proxyReq is skipped when an "except" header is present. Except headers are not supported by undici.

So very promising so far. My next steps would be documentation on the new available options and their implications. And to provide some callback-based alternatives to proxyReq and proxyRes to call before sending the request and after receiving the response.

[Corvince]

I have converted this into a normal pull request, adding a secondary stream path using undici. This code path is triggered by setting the undici option to a truthy value and provides the following options. Its main advantages are full http2 support and better performance.

1. agentOptions that are passed to the creation of the undici Agent that triggeres the requests. This Agent is reused across requests and should thus be quite a bit faster. Also defaults to enabling h2 requests.
2. requestOptions that specify additional request options
3. onBeforeRequest and onAfterResponse. These are alternatives to the "proxyReq" and "proxyRes" events of the http/https code paths.

I updated the documentation to describe these new features - and marked undici code path as experimental.

The undici code path can also be triggered by setting a ´FORCE_UNDICI_PATH´ environmental variable - this is mainly used to run the tests also for the undici code path - only 4 tests are skipped then.

The main point I am hessitant about is the specific undici naming. At first glance I think it might be too implementation specific - something that made http2 support difficult in the first place, because the legacy code is dependent on nodes http/https clients. And users of http-proxy-3 should not need to know that we are using undici. But then again this approach does use undici and the setable agent- and requestoptions are undici specific. And this way users have full control by simply wrapping those.

[williamstein]

I haven't studied the PR enough yet, but I personally like your approach of being explicit about using undici, and also that the http2 functionality is behind that flag (or env variable). I really like the approach you've taken.

I have a question about install size, which just occurred to me. Right now the https://www.npmjs.com/package/http-proxy-3 package is 87 KB, and I think there are some users that really care about the install size. With undici, I guess the install size will increase to at least 2MB, since the install size of undici is 1.47 MB. Have you thought about how to address this?

• I could publish two different packages (but from the same repo, and mention each in the readme)
• something else?

[Corvince]

Thanks for your encouraging words!

Regarding size, that is a tough one and not something I was aware of. But during the night I thought of something else that might also help here. I am still not sure if going undici really is the best way forward. Alternatively I could switch the code to use fetch instead, which is a much more standardized way for, well, fetching data. That means instead of undiciOptions we would have fetchOptions and have a onBeforeRequest with a web-standard Request object and an onAfterResponse with a web standard Response object.
The biggest downside of this would be lacking (default) http2 support, which was the primary point of this whole PR 🤣

But it could also be made to support passing a custom fetch method. So something like this in userland should work

import {fetch, Agent, setGlobalDispatcher} from "undici" 

function myFetch (url, opts) {
  opts ||= {};
  opts.dispatcher = new Agent({allowH2: true});
  return fetch(url, opts);
}

const proxy = createProxyServer({fetch: myFetch})

// Alternatively (but this affects all of fetch)
setGlobalDispatcher(new Agent({allowH2: true})

While a bit cumbersome, this should be more future proof and http2 support in fetch could also be the default in future node.js versions. More importantly this will be independent of the javascript runtime, so for example deno's fetch already supports http2 requests. I just tried a bit testing with bun and bun and undici do not seem to be completely compatible yet.

[williamstein]

Yes, I really like the idea of passing something imported from undici (or whatever) in, given the role of http-proxy-3 as a long-term backward compat not too big library.

[manucorporat]

Can't wait for this to get merged! Amazing job

[williamstein]

@Corvince What's the status?

Also, I wonder if a different approach to avoid blowing up the package size is making undici a "peer dependency"?

@manucorporat It's sitting just because I'm not sure of the status. There was a working version that made http-proxy-3 much larger (due to always installing undici), but then it got refactored to pass undici in... but that maybe doesn't pass the unit tests.

[Corvince]

I had some blockers before, but now this is alive and kicking again! Basic functionality is working again now without undici and just using normal fetch. Need to fix some things still and also update the documentation heavily, which is still using undici. The test errors are somewhat inflated because it is using fetch-api + websocket path, which do not play well together yet. I'll provide some more details tomorrow

[Corvince]

So, this mostly works as intended. Some tests are still failing which need fixing. They belong to 2 categories

1. WebSocket tests. Those should actually not affected at all by the changes of this PR, but I haven't looked into it yet, so not sure what is happening there.
2. (and to be discussed @williamstein). Lots of tests have something like this

    const ports = { source: gen.port, proxy: gen.port };
    const source = http
      .createServer((req, res) => {
        expect(req.method).toEqual("GET");
        expect(req.headers.host?.split(":")[1]).toEqual(`${ports.proxy}`); // <== This is failing
        res.writeHead(200, { "Content-Type": "text/plain" });
        res.end("Hello from " + ports.source);
      })
      .listen(ports.source);

Specifically they test if the "host" header includes the proxy port.
So we are making a request to http://foo:1234 (the proxy) which redirects to http://bar:5678. Now the test expects the host header to be http://foo:1234. But its supposed to be the target host of the request and also a forbidden request header, which means that it is automatically set by fetch to http://bar:5678, which appears correct to me. But this a fundamental difference to how the "normal" path of http-proxy-3 works. So I am not sure how to proceed.

[manucorporat]

We have some big project handling a lot of traffic under http-proxy-3, I will be happy to test some percentage of the traffic under undici, if you guys can ship some sort of pre-release. happy to report and debug any issues that arises.

[Corvince]

To push things forward, I could deactivate the additional tests. The normal functionality works now, especially handling and proxying http2 requests. The additional tests are run with a forced fetch: true setting. Again, some tests fail because the host header is handled differently, which can't be changed. And some websocket tests fail, but we should mark the fetch option as experimental for now anyways. And finally some tests that rely on custom error handling are failing. This needs more more work, but could potentially be handled in a follow up pr.

This would mean I could just update the documentation and deactivate the fetch forced tests and then @williamstein you could do a new release for @manucorporat to test things out. Otherwise it's gonna take some more time, because at least this week I won't be able to work on this

[williamstein]

That sounds like a good plant to me. The key thing is that I just don't want to break http-proxy-3 for any existing users of the current longterm stable api. Adding new not-100% functionality that is only visible with customer configuration is OK, especially if it leads to extra testing.

[Corvince]

Okay, then I will do so tonight or tomorrow. Currently there should be no breaking change to the existing code paths so this should be fine

[Corvince]

I have made the changes, but unfortunately in trying to remove the whitespace changes I introduced even more. The downside of auto-formatting on save in a non-owned repo ;)

[williamstein]

This already looks really good. I have skimmed over everything, but want to take a more careful look tomorrow. If anybody else reading this has any comments or concerns, please let us know. I love that this doesn't directly add new deps, and the new docs with the table are clear.

[Copilot]

Pull Request Overview

This PR adds experimental HTTP/2 support to http-proxy-3 via the fetch API with callback-based request/response lifecycle hooks. The implementation introduces a dual-path architecture where proxying can use either the traditional Node.js native HTTP modules or the new fetch-based approach for HTTP/2 support.

Key Changes:

• New fetch code path with stream2 function supporting HTTP/2 via undici dispatcher
• Callback-based lifecycle hooks (onBeforeRequest, onAfterResponse) for fetch path
• Conditional test skipping to handle path-specific test cases
• Comprehensive documentation for HTTP/2 configuration and usage

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
package.json	Added test scripts for dual-path testing (native and fetch)
lib/index.ts	Changed http import from namespace to type-only import
lib/http-proxy/index.ts	Added FetchOptions interface and fetch configuration support with extensive type refactoring
lib/http-proxy/passes/web-incoming.ts	Implemented stream2 function for fetch-based proxying with HTTP/2 support
lib/http-proxy/passes/web-outgoing.ts	Added HTTP/2 header filtering logic (connection, keep-alive)
lib/http-proxy/common.ts	Added HTTP2_HEADER_BLACKLIST and url property to outgoing options
lib/test/middleware/body-decoder-middleware.test.ts	Wrapped in skipIf to exclude from fetch code path
lib/test/lib/https-proxy.test.ts	Commented out host header port assertions
lib/test/lib/http-proxy.test.ts	Added skipIf conditions for native-only tests, reformatted code
lib/test/lib/http-proxy-passes-web-incoming.test.ts	Added skipIf for proxyReq event tests, updated timeout error expectations
lib/test/http/proxy-http2-to-http2.test.ts	New test suite for HTTP/2 to HTTP/2 proxying
lib/test/http/proxy-callbacks.test.ts	New test suite for fetch callback functions
lib/test/http/error-handling.test.ts	Import formatting change
README.md	Extensive documentation for HTTP/2 support, fetch configuration, and code path compatibility
.github/workflows/test.yml	Updated to run native HTTP tests only

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Poor code formatting with statement on same line as closing brace. The proxy server assignment should be on a separate line for better readability:

});
servers.proxy = proxy.listen(ports.proxy);

Suggested change

	}); servers.proxy = proxy.listen(ports.proxy);
	});
	servers.proxy = proxy.listen(ports.proxy);

[Copilot]

Redundant condition check. The if (err) check is unnecessary since this is inside a catch block where err will always be truthy. The check can be removed:

} catch (err) {
  handleError(err as Error, options.target);
}

Suggested change

	if (err) {
	handleError(err as Error, options.target);
	}
	handleError(err as Error, options.target);

[Copilot]

Missing duplex property for non-GET/HEAD requests with body. According to the Fetch API specification, when using a Request or ReadableStream as the body, you must specify duplex: "half". This is already correctly done for forward requests (line 279), but is missing here. Add:

} else if (req.method !== "GET" && req.method !== "HEAD") {
  requestOptions.body = req;
  requestOptions.duplex = "half";
}

Suggested change

	requestOptions.body = req;
	requestOptions.body = req;
	requestOptions.duplex = "half";

[Copilot]

The updated test now expects either "ECONNRESET" or "UND_ERR_HEADERS_TIMEOUT" error codes. The UND_ERR_HEADERS_TIMEOUT error code is specific to undici (the fetch implementation), which suggests this test may behave differently in the fetch code path. However, the test is not conditionally skipped for either path. Consider whether this test should:

1. Be split into separate tests for native and fetch paths with appropriate error expectations, or
2. Only expect the error code that's common to both paths

The current approach of accepting either error code may mask issues where the wrong code path is being tested.

[Copilot]

Incorrect test file name in comment. The comment references proxy-https-to-https.test.ts but the actual file is proxy-http2-to-http2.test.ts.

Suggested change

	pnpm test proxy-https-to-https.test.ts
	pnpm test proxy-http2-to-http2.test.ts

[Copilot]

Missing parentheses around logical OR expression. This condition is incorrect due to operator precedence. The current code evaluates as:

if (_req.httpVersionMajor > 1 && (key === "connection")) || (key === "keep-alive")

This means key === "keep-alive" will always cause a skip regardless of HTTP version. It should be:

if (_req.httpVersionMajor > 1 && (key === "connection" || key === "keep-alive"))

Both connection and keep-alive headers should only be skipped for HTTP/2+ clients.

Suggested change

	if (_req.httpVersionMajor > 1 && (key === "connection") || key === "keep-alive") {
	// don't send connection header to http2 client
	if (_req.httpVersionMajor > 1 && (key === "connection" || key === "keep-alive")) {
	// don't send connection or keep-alive header to http2 client

[Copilot]

Missing import statement for setGlobalDispatcher. The code example uses setGlobalDispatcher but doesn't import it. Add the import:

import { createProxyServer } from "http-proxy-3";
import { Agent, setGlobalDispatcher } from "undici";

Suggested change

	import { Agent } from "undici";
	import { Agent, setGlobalDispatcher } from "undici";

[Copilot]

Setting a global dispatcher at module level can cause side effects that affect other tests. This test modifies the global state with setGlobalDispatcher, which could interfere with other tests running in the same process. Consider moving this to a beforeAll hook or using a local dispatcher configuration only for this test suite to avoid global side effects.

[williamstein]

I ran an AI review and it did point out some good questions, e.g.,

if (_req.httpVersionMajor > 1 && (key === "connection") || key === "keep-alive") {

looks suspicious. Can you look over those comments? Of course, I also am manually reading the PR.