search

electron / get

Download Electron release artifacts
https://npm.im/@electron/get
MIT License
338 stars 106 forks source link

feat(proxy): support http(s)_proxy sans prefix on nodejs@>=10 #214

Closed BlackHole1 closed 2 years ago

BlackHole1 commented 2 years ago

see: https://github.com/gajus/global-agent/tree/f82ad8a151af30f5bf218487d006771c6407bf89#what-is-the-reason-global-agentbootstrap-does-not-use-http_proxy

What is the reason global-agent/bootstrap does not use HTTP_PROXY?

Some libraries (e.g. request) change their behaviour when HTTP_PROXY environment variable is present. Using a namespaced environment variable prevents conflicting library behaviour.

You can override this behaviour by configuring GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE variable, e.g.

$ export GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE=

Now script initialized using global-agent/bootstrap will use HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment variables.

So ELECTRON_GET_USE_PROXY was actually not working until now...

This PR will contribute to the improvement of upstream applications, e.g: electron-fiddle

close: https://github.com/electron/fiddle/issues/258 https://github.com/electron/fiddle/issues/270

cc: @malept @erickzhao

BlackHole1 commented 2 years ago

When is the code review available? 🙇‍♂️

BlackHole1 commented 2 years ago

Hey @malept 👋. I have optimised the code and added unit tests. The performance of the code is now very much faster than before and the code is also much more simplified. Request re-review

BlackHole1 commented 2 years ago

ping.

BlackHole1 commented 2 years ago

@malept done

BlackHole1 commented 2 years ago

There seems to be a problem with the CI. It is in a Pendding state.

BlackHole1 commented 2 years ago

ping @erickzhao

erickzhao commented 2 years ago

Not sure why the tests aren't being triggered. They seemed to have worked for e5893fc. Maybe push an empty commit to re-kick?

BlackHole1 commented 2 years ago

Not sure why the tests aren't being triggered. They seemed to have worked for e5893fc. Maybe push an empty commit to re-kick?

I tried but to no avail 🤨

malept commented 2 years ago

I think you have to re-log into CircleCI?

BlackHole1 commented 2 years ago

I think you have to re-log into CircleCI?

I don't have access to CircleCI. So logging in again should be useless too

malept commented 2 years ago

You should have access since you're a member of the Electron org.

BlackHole1 commented 2 years ago

You should have access since you're a member of the Electron org.

Wow. you are right. It works fine now

BlackHole1 commented 2 years ago

@malept @erickzhao Can we merge now? I'm ready to submit next PR

electron-bot commented 2 years ago

:tada: This PR is included in version 1.14.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: