Skip to content

perf: use Pool instead of Client in ProxyAgent for HTTP proxy connections #4359

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions lib/dispatcher/proxy-agent.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
'use strict'

const { kProxy, kClose, kDestroy, kDispatch, kConnector } = require('../core/symbols')
const { kProxy, kClose, kDestroy, kDispatch } = require('../core/symbols')
const { URL } = require('node:url')
const Agent = require('./agent')
const Pool = require('./pool')
const DispatcherBase = require('./dispatcher-base')
const { InvalidArgumentError, RequestAbortedError, SecureProxyConnectionError } = require('../core/errors')
const buildConnector = require('../core/connect')
const Client = require('./client')

const kAgent = Symbol('proxy agent')
const kClient = Symbol('proxy client')
Expand All @@ -29,6 +28,7 @@ const noop = () => {}

class ProxyClient extends DispatcherBase {
#client = null
#connector = null
constructor (origin, opts) {
if (typeof origin === 'string') {
origin = new URL(origin)
Expand All @@ -40,7 +40,9 @@ class ProxyClient extends DispatcherBase {

super()

this.#client = new Client(origin, opts)
// Store the connector for direct socket connections
this.#connector = opts?.connect || buildConnector(opts)
this.#client = new Pool(origin, opts)
Copy link
Contributor

@caitp caitp Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new Http1ProxyWrapper does this, although looking at the diff it's kind of clumbsy how it does it -- but the default factory it uses will make a Pool unless opts.connections === 1, which is pulled from the regular Agent factory.

I did switch to using a user supplied factory in order to get MockAgent tests passing, though. maybe it is a good simplification to just always create a Pool, though

}

async [kClose] () {
Expand All @@ -54,7 +56,7 @@ class ProxyClient extends DispatcherBase {
async [kDispatch] (opts, handler) {
const { method, origin } = opts
if (method === 'CONNECT') {
this.#client[kConnector]({
this.#connector({
origin,
port: opts.port || defaultProtocolPort(opts.protocol),
path: opts.host,
Expand Down Expand Up @@ -121,7 +123,7 @@ class ProxyAgent extends DispatcherBase {
if (origin.protocol === 'http:') {
return new ProxyClient(origin, options)
}
return new Client(origin, options)
return new Pool(origin, options)
}
: undefined

Expand Down
Loading