Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dig dug is not connecting with sauce labs correctly when behind http proxy with authentication #17

Closed
bartoszkaczorek opened this issue Jan 7, 2015 · 8 comments

Comments

@bartoszkaczorek
Copy link

bartoszkaczorek commented Jan 7, 2015

In my case I'm under http proxy with authentication and executing functional tests on Sauce Labs with Intern. Dig dug is trying to connect with Sauce Labs API (trying to send job status update) using https and this seems to be a problem when combined with http proxy with authentication. I was able to fix that issue by changing the "SauceLabsTunnel.js" code and using the "request" library from here:
https://github.com/request/request
https://www.npmjs.com/package/request

See here for a updated code snippet that works for me (SauceLabsTunnel.js):

//...
var request = require('request');
//...

sendJobState: function (jobId, data) {

var url = urlUtil.parse(this.restUrl || 'https://saucelabs.com/rest/v1/');
url.auth = this.username + ':' + this.accessKey;
url.pathname += this.username + '/jobs/' + jobId;

payload = JSON.stringify({
build: data.buildId,
'custom-data': data.extra,
name: data.name,
passed: data.success,
public: data.visibility,
tags: data.tags
});

var result = new Promise.Deferred();

request({
body: payload,
headers: {
"accept":"application/json"
},
auth : {
"username" : this.username,
"password" : this.accessKey
},
method : "PUT",
proxy: this.proxy,
url : urlUtil.format(url)

}, function (error, response, body) {
response.data = body;
result.resolve(response);
});

return result.promise.then(function (response) {
if (response.data) {
var data = JSON.parse(response.data);

if (data.error) {
throw new Error(data.error);
}

if (response.statusCode !== 200) {
throw new Error('Server reported ' + response.statusCode + ' with: ' + response.data);
}
}
else {
throw new Error('Server reported ' + response.statusCode + ' with no other data.');
}
});
},
//...

I think the same issue occurs when Dig Dug is trying to download the zip file with sauce connect and it would need to be updated in similar way.
Would it be possible to include those fixes in official release?

Thanks,
Bartosz

@csnover
Copy link
Member

csnover commented Jan 8, 2015

Thanks for your report! An upstream ticket will be opened to add CONNECT tunnelling through a proxy, but we won’t be changing any dependencies.

@lbod
Copy link
Contributor

lbod commented Mar 19, 2015

Hi @csnover, can you let me know if the upstream ticket you mentioned has been added outwith https://bugs.dojotoolkit.org/ticket/18424 ?

I'd like to get some traction on this, whilst the hacking on that ticket worked at home with privoxy, it didn't in my workplace, I'm willing to do more testing/hacking if needed so let me know. I've also pinged @bryanforbes about this

@bartoszkaczorek
Copy link
Author

Can you explain what's wrong with changing dependencies - why invent a wheel when it's already done?

@dylans
Copy link
Contributor

dylans commented Apr 13, 2015

@bartoszkaczorek because we'd rather fix issues with upstream dependencies when possible rather than swapping out an upstream dependency every time there's an issue. There are a number of features in dojo/request that we use for mocking requests, etc., and it wouldn't be very friendly to our users to break those existing patterns. You could argue that anything is a reinvention of something else (e.g. the request library you've linked to isn't the first ever library to solve requests, etc.). :)

@csnover
Copy link
Member

csnover commented Apr 14, 2015

@bartoszkaczorek In addition to what @dylans said, the request library you’re mentioning introduces other dependencies (= slower install), doesn’t follow our standards and conventions (e.g. using Promises for asynchronous operations), and so would require us to rewrite all the networking code in Dig Dug. It’s not a 1:1 replacement. So there are both philosophical and practical reasons why it’s not possible to simply swap it.

@bartoszkaczorek
Copy link
Author

For anyone looking for a workaround for this issue: https://www.npmjs.com/package/digdug-null-sauce-labs-tunnel

@dylans dylans added this to the Dig Dug 2.0 milestone Nov 25, 2016
@morrinene morrinene modified the milestone: Dig Dug 2.0 Nov 30, 2016
@morrinene morrinene assigned bryanforbes and devpaul and unassigned bryanforbes Apr 10, 2017
@morrinene morrinene assigned bryanforbes and unassigned devpaul May 11, 2017
@bryanforbes
Copy link
Member

I have started adding more robust proxy configuration to Dojo as part of dojo/core#335.

@jason0x43
Copy link
Member

Closing this since Intern and digdug no long use dojo for requests. There is a potentially related issue at theintern/intern#1065 if this still happens with current Intern.

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

No branches or pull requests

8 participants