undici icon indicating copy to clipboard operation
undici copied to clipboard

Feature: Rewrite ProxyAgent

Open PandaWorker opened this issue 1 year ago • 4 comments

This would solve...

I want connectors in undici to be responsible for all connections to the destination server. This allows you to configure agents more flexibly.

The implementation should look like...

I wrote a separate package to show how it is necessary and will be more correct to use a proxy

buildHttpProxyConnector buildSocksProxyConnector

import {
  buildHttpProxyConnector,
  buildSocksProxyConnector,
  buildSocksProxyChainConnector
} from '@undicijs/proxy';
import { request, Agent } from 'undici';

// Connectors only for wrapping socket connection for distonation host
const httpProxyConnector = buildHttpProxyConnector('http://3proxy:8080');
const socksProxyConnector = buildSocksProxyConnector('socks5://3proxy:1080');
const socksProxyChainConnector = buildSocksProxyChainConnector(['socks5://3proxy:1080', 'socks5://3proxy:1081']);

// Agents managing of openned sockets and dispatching requests
const httpProxyAgent = new Agent({connect: httpProxyConnector});
const socksProxyAgent = new Agent({connect: socksProxyConnector});
const socksProxyChainAgent = new Agent({connect: socksProxyChainConnector});

Additional context

You can look at it in full at https://jsr.io/@undicijs/[email protected]

I think this would be a better solution. What do you say about this?

PandaWorker avatar Jul 09 '24 20:07 PandaWorker

It was also impossible to implement AsyncDisposable for DispatcherBase. That would solve the try finally problem.

This would solve the problem of cleaning up resources without using try finally.

import { Agent, request } from 'undici';
import { buildHttpProxyConnector } from '@undicijs/proxy';

// Connectors only for wrapping socket connection for distonation host
const httpProxyConnector = buildHttpProxyConnector('http://3proxy:8080');

// disposable agent
class DisposableAgent extends Agent implements AsyncDisposable {
	[Symbol.asyncDispose]() {
		return this.close();
	}
}

/**
 * get ip address with disposable agent via httpProxyConnector
 */
async function getIp() {
	await using dispatcher = new DisposableAgent({
		connect: httpProxyConnector,
		connections: 1
	});

	const ip = await request('https://api.ipify.org/', { dispatcher }).then(
		resp => resp.body.text()
	);

	return { ip };
} // disposing: close socket connections openned with dispatcher (DisposableAgent)

/**
 * get ip address with agent via httpProxyConnector
 */
async function getIpTryFinally() {
	const dispatcher = new Agent({ connect: httpProxyConnector, connections: 1 });

	try {
		const ip = await request('https://api.ipify.org/', { dispatcher }).then(
			resp => resp.body.text()
		);
		return { ip };
	} finally {
		await dispatcher.close();
	}
}

PandaWorker avatar Jul 10 '24 14:07 PandaWorker

The approach you used indeed seems interesting, and I see the benefit of that; nevertheless, I'm not sure a rewrite of the ProxyAgent will be an option here as the agent has a specific purpose and handles several other pieces beyond only connecting to the proxy.

It might be good to document this possibility, as I can see it can be overlooked easily; but not convinced about the rewrite.

About Symbol.asyncDispose, it could be an interesting addition. Stills in stage 3.

metcoder95 avatar Jul 11 '24 07:07 metcoder95

We could implement ProxyAgent using these connector?

ronag avatar Jul 11 '24 08:07 ronag

Yeah, definitely; if we can add a lightweight Proxy that is tight to a single origin, that might help to ease the possibility of having Proxy interceptor as well.

Just the idea of rewriting the current one completely its what I'm not 100% convinced of

metcoder95 avatar Jul 11 '24 20:07 metcoder95