-
Notifications
You must be signed in to change notification settings - Fork 39
feat: Replace agentkeepalive with native Node.js HTTP agents for proxy support
#788
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
Conversation
…y support Replace the agentkeepalive dependency with Node.js native http.Agent and https.Agent to enable HTTP proxy support. The native agents support the HTTP_PROXY and HTTPS_PROXY environment variables out of the box, allowing the client to work seamlessly with proxy configurations. This change maintains the same keep-alive functionality while removing an external dependency and adding proxy support capability.
|
Hi there! Not sure about the right way to get feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the agentkeepalive library with native Node.js HTTP agents to enable proxy support in sandboxed environments. The agentkeepalive module doesn't support proxy configuration, which prevented the Apify CLI from working properly in environments like Claude Code in the browser.
- Removes
agentkeepalivedependency and uses built-inhttp.Agentandhttps.Agentinstead - Maintains keep-alive functionality by explicitly setting
keepAlive: truein agent options - Enables automatic proxy support via standard environment variables (HTTP_PROXY, HTTPS_PROXY)
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/http_client.ts | Replaced agentkeepalive imports with native node:http and node:https modules, updated agent instantiation to use native agents with keepAlive: true, and updated type annotations |
| package.json | Removed agentkeepalive dependency from the dependencies list |
| package-lock.json | Removed agentkeepalive and its transitive dependency humanize-ms, and correctly marked ms as a devDependency since it's only used by dev tools |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // while waiting for the response. | ||
| const agentOpts = { | ||
| const agentOptions: http.AgentOptions = { | ||
| keepAlive: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I've just added commit c424e90 with these optimizations.
Improved native Node.js HTTP/HTTPS agent configuration with: - Disabled Nagle's algorithm (setNoDelay) for lower latency - LIFO socket scheduling to reuse warm connections - Free socket timeout (15s) to prevent connection leaks - Configurable socket pool limits (256 max sockets) - Better keepalive management with timeoutMillis
02e5e04 to
4fd4403
Compare
janbuchar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
barjin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @tducret , please address my comment. Otherwise, it looks good to go to me!
A quick look at the traffic captures shows a similar TCP connection count / timings, so these changes should be safe 👍
| import http from 'node:http'; | ||
| import https from 'node:https'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add both these modules as externals to rsbuild.config.ts (link)? They are not used outside of Node, so we don't need them in the browser bundle. Together, they add around 250kB to the bundle size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing, done in commit f986aa4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Add node:http and node:https to rsbuild externals configuration to prevent bundling these Node.js-only modules in the browser build, reducing bundle size by ~250kB.
|
Thanks for your reactivity ⚡ |
I'm going to release the updated apify-client-js now. Then it depends on how you installed the CLI. If you use the global npm install method, you should be able to just update your global apify-client version to the new one. |

Hello 👋
Here is a PR proposition to be able to use
apifyCLI in a sandboxed environment (in my case, Claude code in the browser).At the moment, we can't do
apify loginorapify infoin such environment, because the apify-client doesn't handle the proxy configuration properly.The commands are stuck.
Digging into it, I found out that the
agentkeepalivemodule doesn't support proxy (see node-modules/agentkeepalive#22 (comment) for instance).I first attempted to use proxy-agents, but it turns out the built-in agents can support both keepalive and proxies out of the box.
So this PR removes
agentkeepalivewhile maintaining the same keep-alive functionality and adding proxy support capability.Test
To test it locally, I created a transparent proxy (we don't intercept requests to avoid self-certificate issues):
mitmdump --listen-port 8000 --ignore-hosts '.*:*' --set dns_log=trueAnd in another terminal, I tested the Apify client with :