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

Expose Undici's ProxyAgent and setGlobalDispatcher within Node #43187

Open
pimterry opened this issue May 23, 2022 · 23 comments
Open

Expose Undici's ProxyAgent and setGlobalDispatcher within Node #43187

pimterry opened this issue May 23, 2022 · 23 comments

Comments

@pimterry
Copy link
Contributor

Originally posted by @pimterry in #42814 (comment)

As a motivating example: I'd love to automatically support global Fetch in https://www.npmjs.com/package/global-agent (drop-in lib to make all Node HTTP use the system proxy) but even though Node bundles Undici AFAICT this isn't possible without adding a full separate Undici dep to that lib, which would more than double its size, just [EDIT: largely] to ship code that's already present in Node itself.

This would be immediately solved if ProxyAgent and setGlobalDispatcher were exposed explicitly somewhere in future.

I've just done a quick test on main in Undici: exposing ProxyAgent & setGlobalDispatcher explicitly increases the Undici bundle size from 334.2kb to 336.0kb (+1.8kb / +0.5%).

Could these APIs be included and exposed in future? Agents for HTTP are a very core API that it would be useful to have usable out of the box, the equivalent functionality is usable OOTB for the legacy http module APIs, and 2kb is not a significant jump in bundle size for this functionality.

@targos
Copy link
Member

targos commented May 23, 2022

setGlobalDispatcher is exposed:

global[Symbol.for('undici.globalDispatcher.1')] = yourDispatcher;

@pimterry
Copy link
Contributor Author

Replying to the last comment from @mcollina in the previous issue:

I would recommend opening up a separate issue about this topic and bringing it to the TSC. I don't think the whole of Undici has the stability guarantees needed to be part of the Node.js LTS cycle yet.

This makes sense! I'm not aware of Undici's process around stability guarantees like this, but I can understand how there are constraints there.

I do understand this isn't a top top priority since installing & importing Undici elsewhere is a usable workaround, so there's certainly no rush just would justify shipping an unstable API unnecessarily. I do think that the current situation isn't a good end state though, and there are good options we can aim for to fix this, so it would be great to find agreement to aim in that direction in the medium term.

I'm not sure how to take anything to the TSC, but very happy to be included on any discussion of this any time.

@pimterry
Copy link
Contributor Author

global[Symbol.for('undici.globalDispatcher.1')] = yourDispatcher;

That's does work as a workaround, but it's not especially nice as the official API for this, and I think ProxyAgent needs to be available too to make this usable for global Fetch.

@mcollina
Copy link
Member

I don't think we should expose undici further until we want to consider fetch as stable.

@frank-dspeed

This comment was marked as off-topic.

@mcollina

This comment was marked as off-topic.

@sosoba
Copy link
Contributor

sosoba commented Dec 5, 2022

@mcollina
I don't think we should expose undici further until we want to consider fetch as stable.

2022-11-14, Version 19.1.0

@mcollina
Copy link
Member

mcollina commented Dec 5, 2022

That does not mark it stable, on purpose. It's the first step on that journey. This is something we might want to consider right now.

@mcollina
Copy link
Member

In the TSC meeeting of today, we decided that we are going to support HTTP_PROXY, HTTPS_PROXY and NO_PROXY env variables directly in Undici: nodejs/undici#1650.

@mcollina mcollina removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Jan 18, 2023
@GabenGar
Copy link

How is it going to handle different casing rules?

@JasonKleban
Copy link

setGlobalDispatcher is exposed:

global[Symbol.for('undici.globalDispatcher.1')] = yourDispatcher;

Is there a workaround to obtain ProxyAgent?

@ert78gb
Copy link

ert78gb commented Nov 17, 2023

Hi All,

I would like if you expose the whole undici.

  • I don't have to install undici if I liked to use feature which is available only in it
  • the whole mocking functionality is available when I write test for the fetch API.

If you expose only a certain feature of undici then there will be someone who needs some unexposed feature, so better to expose everything in 1 step.

I think it is not worth polluting the global namespace so I suggest using node:undici.

@silverwind
Copy link
Contributor

silverwind commented Feb 22, 2024

While I would like to see ProxyAgent exposed too, I believe that it'd be even better if a proxy option would be added into fetch itself.

With this, node's fetch would diverge from web fetch, but imho for good reason because the option is not relevant in browsers where proxies are handled outside of JS.

@mcollina mcollina added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Feb 22, 2024
@mcollina
Copy link
Member

I think it's time for the TSC to make some plans about this. I'll cover this at the collab summit too.

@silverwind
Copy link
Contributor

silverwind commented Mar 16, 2024

In addition to ProxyAgent it would also be nice to expose Agent. I currently use it to set keepAlive and connections options in undici though it that are not exposed in fetch.

@timtucker-dte
Copy link

In addition to ProxyAgent it would also be nice to expose Agent. I currently use it to set keepAlive and connections options in undici.

We're using HttpAgent similarly now so that we can override DNS resolution with our own caching resolver.

@longzheng
Copy link

setGlobalDispatcher is exposed:

global[Symbol.for('undici.globalDispatcher.1')] = yourDispatcher;

In case anyone else was looking for a way to overriding the global dispatcher with a new agent with some custom config options, here's an example

    // there is no official Node API to get the global dispatcher (as of Node 20.11.1)
    // getting the dispatcher from global is considered a public API https://github.com/nodejs/undici/discussions/2167#discussioncomment-6265039
    // hard-coded symbol for the undici global dispatcher https://github.com/nodejs/undici/blob/main/lib/global.js#L5
    const unidiciGlobalDispatcherSymbol = Symbol.for('undici.globalDispatcher.1');
    const undiciGlobalDispatcher = global[unidiciGlobalDispatcherSymbol];

    if (!undiciGlobalDispatcher) {
        throw new Error('Could not find the global Undici dispatcher, maybe not in node runtime?');
    }

    // initialize a new agent/dispatcher https://undici.nodejs.org/#/docs/api/Agent
    // inspired by https://github.com/orgs/nodejs/discussions/51260#discussioncomment-7949240
    const newDispatcher = new undiciGlobalDispatcher.constructor({
        // limit the number of client (host) connections
        // https://undici.nodejs.org/#/docs/api/Pool
        connections: 128,
    });

    // override the Undici global dispatcher
    global[unidiciGlobalDispatcherSymbol] = newDispatcher;

@mbrevda
Copy link

mbrevda commented May 2, 2024

so, there is no way to set a dispatcher on a per-call basis yet?

@silverwind
Copy link
Contributor

silverwind commented May 2, 2024

so, there is no way to set a dispatcher on a per-call basis yet?

You can already set dispatcher in fetch options. But to really make it useful, Agent and ProxyAgent need to be accessible without importing them from undici.

@mbrevda
Copy link

mbrevda commented May 2, 2024

But to really make it useful, Agent and ProxyAgent need to be accessible without importing them from undici.

right. @mcollina any updates from TSC?

@mcollina
Copy link
Member

mcollina commented May 2, 2024

There is no consensus on exposing more of undici.

My current plan is to add support for the HTTP_PROXY env variable behind a flag in v22 (and possibly v20), then unflag in v23.

@JasonKleban
Copy link

@mcollina reading HTTP_PROXY is probably not powerful enough. I have (dev only, for use with Fiddler) a loop that monitors for the proxy server being up and only then enrolling requests in the proxy. This allows the use of Fiddler to be decided for an already running process and not depend on Fiddler running for calls to succeed.

I don't suggest that precisely this better but still imperfect mechanism be built into node - but we ought to be able to write our own. Exposing the primitives for this is better than choosing one underpowered implementation for all.

@mcollina
Copy link
Member

mcollina commented May 2, 2024

Agreed. If you need that level of sophistication you are better off in using undici directly for now.

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

Successfully merging a pull request may close this issue.