Make it possible to use Fetch with proxies or other agents #42814

@nodejs/undici

Note: the same applies to undici's mocking ability and common userland modules that achieve mocking for node's http/https modules nock. Undici has the affordance, but it is not exposed in Node.js and so node's global fetch cannot be mocked.

fetch in Node.js is experimental, all of this is planned for when it exits experimental. Could you check out undici and see if it matches your expectations?

On a different note, would it be easier to detect if the global fetch property was non-enumerable?

Could you check out undici and see if it matches your expectations?

Undici's dispatchers & setGlobalDispatcher on the core request methods (request/stream/etc) do work great for me, yes.

Undici's fetch doesn't have its own dispatcher option available though (nodejs/undici#1350), which is a bit inconvenient (I'm mostly setting a global dispatcher anyway, so in my case it's not a big issue).

It would be a little more useful if it directly supported agents, or there was a dispatcher<->agent compatibility wrapper, since there's so many existing node.js agent implementations that could be reused, but again not a big deal.

On a different note, would it be easier to detect if the global fetch property was non-enumerable?

I don't think enumerability matters here, most checks are just if (!global.fetch) { addPolyfill() } e.g. in isometric-fetch and cross-fetch.

Those libraries are quite widely used (5 million + 8 million installs a week via npm, plus other similar libs & inline checks elsewhere). Everybody using those with Node 18 is automatically using this new experimental implementation everywhere now.

That's how I ran into this myself: my test code had the same inline if-no-fetch-then-ponyfill check, and when I ran my tests in node 18 they all failed because they switched to node's fetch and started ignoring the configured HTTP proxy.

@nodejs/tsc did we expose fetch too early?

@nodejs/tsc did we expose fetch too early?

Not sure. It's difficult to say if we would have gotten useful feedback with the API still behind a flag.

@pimterry note that you can opt out of node's fetch with --no-experimental-fetch.

@nodejs/tsc did we expose fetch too early?

The problem isn't exposed fetch. It's that it doesnt utilize node's global agents due to its use of undici. There's a whole ecosystem of mocking and proxying libraries that relied on the fact that regardless of what http client / library you used in node, it always boiled down to http/s.request and its use of global agents

fetch in Node.js is experimental, all of this is planned for when it exits experimental

In hindsight, a global that implements a Web Platform API that's actively being polyfilled probably shouldn't be exposed when still experimental / unstable. That being said

It's difficult to say if we would have gotten useful feedback with the API still behind a flag.

Exposing it via a node:fetch module first would probably allow for more feedback to trickle in. WebCrypto API was also not exposed as global and was first added to the crypto module as an export while experimental.

https://github.com/orgs/nodejs/teams/tsc did we expose fetch too early?

Maybe. But I'm not sure what waiting even more would have made any difference? The same things break. The polyfills are still using node streams and http global agents so the primary breakage would be the same.

I think we should seriously consider exposing node:undici, or possibly node:http-next. What do you think?

I like the idea!

This isn’t hindsight, to be clear; the risk was brought up many times and intentionally gambled.

Ah, that's interesting! I think the point largely stands though, since adding the dependency does duplicate the vast majority of Undici's other code - lib/fetch and lib/llhttp alone are more than 50% of the size of the Undici package, and are both included in node's bundle. Needing to install & import all that code that's already present is suboptimal.

I briefly had a go at creating an Undici fork that contained only ProxyAgent & setGlobalDispatcher, but Undici's agent.js depends on client.js which is a full HTTP client implementation, so there's no way to avoid this that way.

On the flip side though, that overlap suggests that ProxyAgent & setGlobalDispatcher would probably not significantly increase the size of Undici in Node. Looking at the Undici.js bundle (I assume that's the right place?) I can see that the entirety of setGlobalDispatcher, and the Dispatcher, DispatcherBase and Agent classes are already included there. It's only ProxyAgent itself that's missing, which is tiny.

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.

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.

@mcollina Moved into a new issue here: #43187

For any lost souls, this is my understanding of how to use custom certs for node:http, node:https, and undici.fetch:

import { globalAgent } from "node:https";
import { getCACertificates } from "node:tls";
import { Agent, setGlobalDispatcher } from "undici";

const caCerts = [
  ...getCACertificates("system"),
  ...getCACertificates("bundled"),
];

// Get node:http and node:https to use both the system CA certs and the bundled
// (Mozilla) CA certs.
//
// node-fetch (not to be confused with node:fetch) is based on node:http and
// node:https, so this should handle that as well.
//
// - https://github.com/electron/electron/issues/45674
globalAgent.options.ca = caCerts;

// Get node:fetch to do the same by reconfiguring the underlying `undici`
// library (as node:fetch is not written on top of node:http and node:https).
// - https://github.com/nodejs/undici#undicisetglobaldispatcherdispatcher
const agent = new Agent({ connect: { cert: caCerts } });
setGlobalDispatcher(agent);

// Now all fetch() usages imported from undici from this point onwards will use
// our custom certs. Though be aware that fetch() usages imported from
// node:fetch will not be affected by this reconfiguration. In other words:
//
// ```js
// import { fetch } from "undici";
//
// // This will use the custom certs we set on `globalAgent.options.ca`.
// await fetch("https://bbc.co.uk");
// ```
//
// ```js
// import { fetch } from "node:fetch";
//
// // This will *not* be affected by the custom certs we set on
// // `globalAgent.options.ca`. It will continue to use the (Mozilla) CA certs
// // that come bundled with Node.js.
// await fetch("https://bbc.co.uk");
// ```

At the time of writing, there is no node:undici library, so you do have to install undici separately to get this level of customisation. There is TSC support for exposing such a thing in future, it just lacks a champion for now.

Also, if you have to rely on native fetch in combination with the Agent from undici, it might make sense to have a unit test like that to avoid any potential subtle compatibility issues because of version mismatch:

it('verify that we use exactly the same `undici` version at both the application and runtime levels', async () => {
  expect(process.versions.undici).toEqual(require('undici/package.json').version);
});