add proxy support for download now-cli #1844
Conversation
Codecov Report
@@ Coverage Diff @@
## canary #1844 +/- ##
======================================
Coverage 6.34% 6.34%
======================================
Files 93 93
Lines 4049 4049
======================================
Hits 257 257
Misses 3792 3792Continue to review full report at Codecov.
|
download/src/index.js
Outdated
| @@ -9,6 +9,7 @@ import fetch from 'node-fetch'; | |||
| import retry from 'async-retry'; | |||
| import which from 'which-promise'; | |||
| import readPkg from 'read-pkg'; | |||
| import HttpsProxyAgent from 'https-proxy-agent'; | |||
There was a problem hiding this comment.
It would be better to use the proxy-agent module for this, so that SOCKS proxies would also be supported.
download/src/index.js
Outdated
| @@ -86,7 +87,18 @@ async function download() { | |||
| try { | |||
| const name = platformToName[platform]; | |||
| const url = `https://github.com/zeit/now-cli/releases/download/${packageJSON.version}/${name}.gz`; | |||
| const resp = await fetch(url, { compress: false }); | |||
| // check whether the proxy was set and configure the proxy agent settings | |||
There was a problem hiding this comment.
The proxy-agent module also does the env var parsing logic. See TooTallNate/node-proxy-agent#16.
package.json
Outdated
| @@ -206,5 +206,8 @@ | |||
| "utility-types": "2.1.0", | |||
| "which-promise": "1.0.0", | |||
| "write-json-file": "2.2.0" | |||
| }, | |||
| "dependencies": { | |||
| "https-proxy-agent": "2.2.1" | |||
There was a problem hiding this comment.
The proxy-agent module should be in devDependencies like the other deps.
There was a problem hiding this comment.
Yeah, The proxy-agent module is more powerful and graceful, but it maybe hide more details in my opinion. Need more discussion to choose upon advantages and disadvantages.
I saw the comments above, and I will change the proxy strategy and update the pr. |
|
Hey @liudonghua123, could you please take a look at the failing tests ^? 🙏 |
|
Thank you for your suggestion! Unfortunately, since this PR seems to be unmaintained, we will close it for now. However, the team will still be tracking the scenario where Now CLI is not able to install behind a intranet proxy. |
|
Hi, @juliangruber I didn't found any error checks or failing tests. Could you show me the steps to see the failing tests during the CI process. Thanks. |
If I was in a intranet environment, I could not download and install now-cli successfully.