Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

sauceAgent passed incorrectly to SauceLabs.js #3410

Closed
wyvern8 opened this issue Jul 25, 2016 · 3 comments · Fixed by #3415
Closed

sauceAgent passed incorrectly to SauceLabs.js #3410

wyvern8 opened this issue Jul 25, 2016 · 3 comments · Fixed by #3415
Assignees

Comments

@wyvern8
Copy link

wyvern8 commented Jul 25, 2016

It looks like there is an issue with the way the sauceAgent config param is passed to the SauceLabs node module. It is expecting 'proxy' to be passed in rather than 'agent'.

the sauceAgent config is a string according to this which is fine:
https://github.com/angular/protractor/blob/master/lib/config.ts

..and it looks like the intent is that this is the proxy address to be passed to SauceLabs node module when behind a proxy - docs are a bit unclear.

when we just set the webDriverProxy, the tests run against saucelabs via the proxy ok, but the final result rest api call fails (outside webdriver) - even though we have the sauceAgent proxy set too. This means that all tests are left in an error state.

When i locally update this file to match the expected constructor parameter it works fine:

https://github.com/angular/protractor/blob/master/lib/driverProviders/sauce.ts

from this:
this.sauceServer_ = new SauceLabs({
username: this.config_.sauceUser,
password: this.config_.sauceKey,
agent: this.config_.sauceAgent
});

to this:
this.sauceServer_ = new SauceLabs({
username: this.config_.sauceUser,
password: this.config_.sauceKey,
proxy: this.config_.sauceAgent
});

this means that the parameter matches the expected constructor parameter - 'agent' does not seem to be referenced in SauceLabs.js:

https://github.com/danjenkins/node-saucelabs/blob/master/lib/SauceLabs.js

this options is then used to setup the proxy like this:
options.agent = new HttpsProxyAgent(url.parse(options.proxy));

happy to provide more details if required.

@wyvern8
Copy link
Author

wyvern8 commented Jul 25, 2016

looking closer at this, is perhaps sauceAgent meant to be passed though as 'agent' as an HttpsProxyAgent rather than string?

it seems SauceLabs.js supports setting the proxy as a string as well, rather than having to create an agent.

if(options.proxy){
options.agent = new HttpsProxyAgent(url.parse(options.proxy));
}

however when i try setting up an agent in this manner and then passing it in as 'sauceAgent' i get the below - possibly something on our side, but proxy string would be simpler - sauceProxy->proxy ?:

TypeError: self.agent.addRequest is not a function
[INFO] [chrome 51.0 Windows 10 #1-0] at new ClientRequest (_http_client.js:158:16)
[INFO] [chrome 51.0 Windows 10 #1-0] at Object.exports.request (http.js:31:10)
[INFO] [chrome 51.0 Windows 10 #1-0] at Object.exports.request (https.js:199:15)
[INFO] [chrome 51.0 Windows 10 #1-0] at Object.request (/home/cq/dev/workspace/common_app/content/node_modules/agent-base/patch-core.js:52:20)
[INFO] [chrome 51.0 Windows 10 #1-0] at makeRequest (/home/cq/dev/workspace/common_app/content/node_modules/saucelabs/lib/SauceLabs.js:279:25)
[INFO] [chrome 51.0 Windows 10 #1-0] at SauceLabs.send (/home/cq/dev/workspace/common_app/content/node_modules/saucelabs/lib/SauceLabs.js:250:3)
[INFO] [chrome 51.0 Windows 10 #1-0] at SauceLabs.updateJob (/home/cq/dev/workspace/common_app/content/node_modules/saucelabs/lib/SauceLabs.js:102:8)

@cnishina
Copy link
Member

Without creating breaking changes, I think we should do the following:

proxy: this.config_.sauceProxy,
agent: this.config_.sauceAgent

I'll try this out and put a pull request together.

Good catch!

@wyvern8
Copy link
Author

wyvern8 commented Jul 27, 2016

Thanks @cnishina - much appreciated!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants