Conversation
BREAKING CHANGE: Proxy environment variables now used.
…erRequest BREAKING CHANGE: Proxy env vars now used
| - Domain suffixes: `.local`, `.internal.example.com` | ||
| - Comma-separated lists: `localhost,.local,internal.example.com` | ||
|
|
||
| Proxy detection uses the [`proxy-from-env`](https://www.npmjs.com/package/proxy-from-env) library, which follows standard proxy environment variable conventions. |
There was a problem hiding this comment.
Considering that axios has proxy support out-of-the-box using proxy-from-env, what does this PR add other than being able to overwrite the env variables via parameters?
I would at least assume that's not a breaking change based on that?
Or does escher-request set any parameters that somehow disable axios' ootb support? Like http(s)Agent?
Can't really dig in as I'm OOO.
There was a problem hiding this comment.
Good find. We do use http/sAgent to support keep alive. Perhaps we only use this new logic for that use case since it is already supported otherwise. I believe there is a known issue for keep alive.
There was a problem hiding this comment.
The axios docs even mention that keepAlive is enabled by default since Node.js v19 and our lowest supported version is 20.
Maybe there is some potential for cleanup, but that's for another PR.
There was a problem hiding this comment.
To be honest, I only maintain this repository to keep its dependencies up-to-date, but didn't really write or touch the actual code myself.
So I'm not really sure how to judge this change...
There was a problem hiding this comment.
axios.get('https://www.google.com', {
httpsAgent: new HttpsProxyAgent('http://localhost:3128'),
// proxy: false
})❯ env HTTPS_PROXY=http://localhost:3128 node index.js
Request failed with status code 502
❯ env NO_PROXY=www.google.com HTTPS_PROXY=http://localhost:3128 node index.js
200 <!doctype html><html itemscope="" itemtype="http://schema.org/WebPage" lang="de-AT"><head><meta content="text/html; charset=UTF-8" http-equiv="Content-Type"><meta content="/images/branding/googleg/1x/ axios.get('https://www.google.com', {
httpsAgent: new HttpsProxyAgent('http://localhost:3128'),
proxy: false
})❯ env HTTPS_PROXY=http://localhost:3128 node index.js
200 <!doctype html><html itemscope="" itemtype="http://schema.org/WebPage" lang="de-AT"><head><meta content="text/html; charset=UTF-8" http-equiv="Content-Type"><meta content="/images/branding/googleg/1x/no_proxy itself is working apparently, but because the https is broken the internal proxy mechanism must be disabled, this is the core of the issue :/
There was a problem hiding this comment.
f0a947f generally disables axios' broken built-in support.
Not 100% sure if this is desired, though.
| return new RequestWrapper(requestOptions, protocol, payload); | ||
| } | ||
|
|
||
| private getOptionsFor(method: string, path: string): ExtendedRequestOption { |
There was a problem hiding this comment.
see my comment below about proxy: false
SchroederSteffen
changed the title
3 participants
BREAKING CHANGE: uses proxy environment variables if available to connect through the proxy