Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Rewrite ProxyAgent #3397

Open
PandaWorker opened this issue Jul 9, 2024 · 4 comments
Open

Feature: Rewrite ProxyAgent #3397

PandaWorker opened this issue Jul 9, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@PandaWorker
Copy link

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 PandaWorker added the enhancement New feature or request label Jul 9, 2024
@PandaWorker
Copy link
Author

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();
	}
}

@metcoder95
Copy link
Member

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.

@ronag
Copy link
Member

ronag commented Jul 11, 2024

We could implement ProxyAgent using these connector?

@metcoder95
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants