Added HTTP Proxy Support #101
Conversation
index.js
Outdated
|
|
||
| console.assert(params.token, 'token must be defined'); | ||
|
|
||
| // Use a HTTP Proxy if defined in the environment. We need to accomodate both uppercase and lowercase environment definitions |
There was a problem hiding this comment.
Just as sugestion, this could be refactored into an encapsulated proxy setup method or similar :)
There was a problem hiding this comment.
Done. As a bonus, defining a proxy param in the constructor will be respected before looking at the environment.
index.js
Outdated
| // Use a HTTP Proxy if defined in the environment. We need to accomodate both uppercase and lowercase environment definitions | ||
| this._agent = null; | ||
| if ((typeof process.env.http_proxy !== 'undefined' && process.env.http_proxy) || (typeof process.env.HTTP_PROXY !== 'undefined' && process.env.HTTP_PROXY)) { | ||
| this._agent = new HttpsProxyAgent(Url.parse(process.env.http_proxy || process.env.HTTP_PROXY)); |
There was a problem hiding this comment.
What will happen if Url.parse return error?
There was a problem hiding this comment.
Have updated to code handle any throws from Url.parse and to check if the parse didn't fail but didn't succeed either :/
|
First Github pull request ever @jc21! :) |
|
That latest CI failure is compilation related, timed out trying to install nvm. Can you re-trigger? |
|
Is there any progress with this PR as this is to something that I'd be keen to utilise as well? |
|
@jesuslg123 is there any movement on this being merged into the project please? |
… proxy-support
5 participants
Fixes #75
Tested on environments both with and without a proxy environment variable suitably defined.
Thanks to @rayterrill for the initial work on this.