Allow passing request context to PoolOptions

[blinsay]

This would solve...

When creating a new connection, I'd like to do a host lookup based on multiple parts of a request and create a new TLS connection. Sometimes the name of the cert is based on the origin in the URL, sometimes request headers need to be taken into account, and so on.

The implementation should look like...

PoolOptions.factory should take DispatchOptions as a third argument. It could pass the entire request in as a third argument here.

https://github.com/nodejs/undici/blob/03c98d5a683df54c47ee49b8db0ffd7d56f45525/lib/dispatcher/agent.js#L78-L88

This is a public API change that exposes a lot more data to PoolOptions.factory, so I didn't want to open a PR without checking with folks first. The diff to agent.js could be nice and small, but I'm not sure what else I'd have to do to make the change.

    if (!dispatcher) {
-     dispatcher = this[kFactory](opts.origin, this[kOptions])
+     dispatcher = this[kFactory](opts.origin, this[kOptions], opts)
        .on('drain', this[kOnDrain])
        .on('connect', this[kOnConnect])
        .on('disconnect', this[kOnDisconnect])
        .on('connectionError', this[kOnConnectionError])

      // This introduces a tiny memory leak, as dispatchers are never removed from the map.
      // TODO(mcollina): remove te timer when the client/pool do not have any more
      // active connections.
      this[kClients].set(key, dispatcher)
    }

I have also considered...

I've considered overriding Agent with a completely custom class to do this - it'd be a lot of work, but it would be nice to write less code and use the hook provided on PoolOptions.

Thanks!

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[blinsay]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mcollina sure! Before I get started, is this something you all would be interested in? Is the API change a concern?

[Ethan-Arrowood]

While we cannot guarantee the feature will land, its much easier to get consensus from maintainers if they have an implementation to look at. I think what you are adding here shouldn't be too major. I recommend going ahead with a solution that solves your use case and we can discuss in the PR 😄