-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(webdriver-manager): use proxy for webdriver-manager #966
Conversation
I tests against http://www.charlesproxy.com/ already (since I don't have corp proxy), but those who need this feature please try it and let me know if it doesn't work for you. |
if (argv.proxy) { | ||
return argv.proxy; | ||
} else if (protocol === 'https:') { | ||
return process.env.HTTPS_PROXY || process.env.HTTP_PROXY; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to support this, or require it to be explicitly entered with --proxy
? I haven't had to work behind a corporate proxy, but after a quick search using env.HTTPS_PROXY doesn't seem to be a common pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
responding to myself. Looked around, did find a couple uses of this, so let's keep it.
LGTM |
merged with 6906c93 |
I've made a couple of minor modifications here: #968 The error handling for the request was not evaluating the I also added support for both lower case and upper case environment variables, i.e. (HTTPS_PROXY and https_proxy). This is in line with what tools like npm use. |
at first it didnt work for me. |
Doesn't work for a couple of us. We tried |
No description provided.