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

http: deal with the lack of http proxy support in node.js core. #15620

Open
martinheidegger opened this Issue Sep 26, 2017 · 8 comments

Comments

Projects
None yet
7 participants
@martinheidegger
Copy link

martinheidegger commented Sep 26, 2017

This is a follow-up on #8381.

Summary:
Node's http module does not support http proxies by default. This is a unfortunate situation as libraries don't use packages for http that support http proxies and as a result quite a few tools in the Node ecosystem lack the support for http proxies (i.e. through environment variables).

It would be helpful if Node.js could support user proxies and if someone could take the time to implement it, I think the following plan could help get us there:

  1. Old versions of Node.js will never gain http proxy support so it would be important to have a recommended way how to add proxy support to user-land libraries to work on the old versions of Node.js. A guide on "how to support proxies" (maybe combined with a package) seems like a minimum requirement to fix this issue which will help no matter how this topic progresses. (Anyone can start with this, no need to wait for approval, help dearly welcome).
  2. Create a PR to the Node.js docs in which it clearly explains that proxies are the responsibility of user-land and reference the recommended implementation/article in 1. (Just reflecting the current state of affairs)
  3. Create a new discussion on to how the package used in 1. could become part of Node.js core in order to reduce the implementation cost for current and new packages.
  4. Discuss the possibility of creating a Node flag that enables the behavior of 3. in case someone is stuck with user-land code that doesn't implement proxies.

@martinheidegger martinheidegger changed the title http: improve http proxy problematic http: deal with the lack of http proxy support in node.js core. Sep 26, 2017

@hiroppy hiroppy added the http label Sep 26, 2017

@mscdex

This comment has been minimized.

Copy link
Contributor

mscdex commented Sep 26, 2017

I'm not keen on adding HTTP proxy support in core, especially because it could be a slippery slope (e.g. people may ask to also add support for SOCKS, etc.). IMHO this is easily done in userland with a custom http.Agent. If you want it to be more automatic for all HTTP requests, you can just overwrite the http.globalAgent with your custom implementation.

@martinheidegger

This comment has been minimized.

Copy link
Author

martinheidegger commented Sep 26, 2017

@mscdex The point of this issue is to bikeshed this particular discussion until point 3.

@Fishrock123

This comment has been minimized.

Copy link
Member

Fishrock123 commented Sep 26, 2017

Old versions of Node.js will never gain http support so it would be (...)

Did you mean "never gain http proxy support"?

@martinheidegger

This comment has been minimized.

Copy link
Author

martinheidegger commented Sep 26, 2017

@crosscompile

This comment has been minimized.

Copy link

crosscompile commented Jan 9, 2018

I'm not sure this is the kind of implementation that people had in mind, but after reading through #8381 I wrote a tiny lib env-proxy-agent that just wraps proxy-from-env and proxy-agent.

I wasn't able to come up with a solution that could overwrite http.globalAgent as I think NO_PROXY needs to be matched against each request individually. This is probably due to my own lack of knowledge about http.Agent but if anyone has guidance on the issue that would be great.

@stevenvachon

This comment has been minimized.

Copy link

stevenvachon commented Jan 9, 2018

@crosscompile why write a separate project when you could've instead fulfilled TooTallNate/node-proxy-agent#11

@crosscompile

This comment has been minimized.

Copy link

crosscompile commented Jan 9, 2018

@stevenvachon thanks for pointing that out, I did not see that open issue on node-proxy-agent.

I pushed up some changes to my fork of node-proxy-agent that I'm hoping to finish up soon, will follow up in TooTallNate/node-proxy-agent#11 as it's more relevant there.

@chrmarti

This comment has been minimized.

Copy link

chrmarti commented Oct 2, 2018

Overwriting http.globalAgent as suggest above does not work (#9057). One has to pass the agent option to each and every request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment