nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.08k stars 530 forks source link

Feature: Rewrite ProxyAgent #3397

Open PandaWorker opened 2 months ago

PandaWorker commented 2 months ago

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/proxy@0.2.1

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

PandaWorker commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

We could implement ProxyAgent using these connector?

metcoder95 commented 2 months ago

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