Add HTTPS_PROXY environment variable support to Python and JS SDKs#1270
Add HTTPS_PROXY environment variable support to Python and JS SDKs#1270malthejorgensen wants to merge 2 commits intoe2b-dev:mainfrom
Conversation
This adds support for the following proxy environment variables: - https_proxy - HTTPS_PROXY - http_proxy - HTTP_PROXY - all_proxy - ALL_PROXY Lowercase version takes precedence as that seems to be standard across cURL, Python, and wget -- yes, I agree -- it is nuts! For a reference on the used environment variables and their precedence see: https://superuser.com/a/1690537
malthejorgensen
requested review from
ValentaTomas,
jakubno and
mishushakov
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c37194891
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { ProxyAgent } = require('undici') |
There was a problem hiding this comment.
getProxyFetch() relies on require('undici') to create ProxyAgent, but packages/js-sdk/package.json does not declare undici as a dependency, so typical installs of this SDK in Node will hit MODULE_NOT_FOUND and immediately fall back to plain fetch. In that scenario, setting HTTPS_PROXY/HTTP_PROXY has no effect, so the new proxy-support behavior is silently non-functional.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Add the undici dependency.
This adds support for the following proxy environment variables: - https_proxy - HTTPS_PROXY - http_proxy - HTTP_PROXY - all_proxy - ALL_PROXY Lowercase version takes precedence as that seems to be standard across cURL, Python, and wget -- yes, I agree -- it is nuts! Adds the dependency `undici` which allows `fetch` requests to go through a proxy. `undici` is owned by the Node.js organization (https://github.com/nodejs/undici). For a reference on the used environment variables and their precedence see: https://superuser.com/a/1690537
malthejorgensen
force-pushed
the
add-https-proxy-envvar-support
branch
from
6c37194 to
0019f61
Compare
|
Claude's own description of the PR: Summary
DetailsWhy proxy env vars were ignored:
Env vars checked (both SDKs): Explicit Test plan
|
1 participant
tl;dr Claude Code's web environment passes all requests through the proxy set in the environment variable
HTTPS_PROXY. Since the E2B SDKs don't respect that variable, Claude Code on the web can't call the E2B APIs.I've run formatters, linters and tests in both the Python and JS SDKs. As far as I can tell I'm only getting errors unrelated to this PR.
I've tested both the Python SDK and JS SDK in Claude Code web. Without this PR, Claude (on the web) is unable to interact with E2B sandboxes, and with this PR it is able to.
I'm more of a Python programmer. For the JS piece we need the Node official https://github.com/nodejs/undici to get the proxy support -- so it adds an extra dependency to the JS SDK.
This PR adds support for 6(!) different environment variables and I'd usually consider that AI slop, but it does seem to follow what other tools support (cURL, Python, and wget) with lowercase variables taking precedence: https://superuser.com/a/1690537
The code was generated with Claude Code, but I have read and tested it and made some smaller adjustments to both the JS SDK code and Python SDK code.