perf: use Pool instead of Client in ProxyAgent for HTTP proxy connections#4359
Closed
mcollina wants to merge 1 commit into
Closed
perf: use Pool instead of Client in ProxyAgent for HTTP proxy connections#4359mcollina wants to merge 1 commit into
mcollina wants to merge 1 commit into
Conversation
…ions - ProxyClient now uses Pool instead of Client for better connection management - Added connector storage for direct CONNECT method handling when tunneling is disabled - Updated factory function to use Pool for non-tunneling HTTP-to-HTTP connections - Maintains backward compatibility for both tunneling and non-tunneling scenarios - Removes unused imports (kConnector, Client) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matteo Collina <hello@matteocollina.com>
Member
Author
|
cc @caitp (I know this now conflicts) |
caitp
reviewed
Jul 24, 2025
| 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) |
Contributor
There was a problem hiding this comment.
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
Member
Author
There was a problem hiding this comment.
Then this PR is not needed
metcoder95
approved these changes
Jul 24, 2025
Member
metcoder95
left a comment
metcoder95
left a comment
There was a problem hiding this comment.
lgtm, just conflicts arised after previous PR merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Benefits
Changes
ProxyClientconstructor now usesnew Pool()instead ofnew Client()#connectorfield to store raw connector for CONNECT method handlingkConnector,Client)Test Plan
🤖 Generated with Claude Code