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

node-xhr2 behind a proxy - food for thoughts #1

Closed
asnowfix opened this issue Jan 30, 2013 · 7 comments
Closed

node-xhr2 behind a proxy - food for thoughts #1

asnowfix opened this issue Jan 30, 2013 · 7 comments

Comments

@asnowfix
Copy link
Contributor

I am working on this topic right now.

Browser's XMLHttpRequest objects do not have an API to set an egress proxy, because everything is cabled by the Browser's internals. Node-speaking, the HTTP & HTTPS Agents are managed by the Browser, not by the XMLHttpRequest root object.

I think it would be good to keep the same principles, by adding a setAgents method (or an agents construction time-option) rather than a setProxy. The HTTP and/or HTTPS agents would be created by the upper layer (dropbox-js in this case), depending on the needs.

Opinion?

@pwnall
Copy link
Owner

pwnall commented Jan 30, 2013

How about using node_httpAgent and note_httpsAgent properties to set the Agents? You can set them on an instance-by-instance basis, or you can change them in XMLHttpRequest.prototype

Would this work?

@asnowfix
Copy link
Contributor Author

This is what I had in mind: In my locally modified xhr2, this is what I am doing, with the following change:

  1. I have created an external interface construct(options.agents) and setAgents() to avoid fiddling directly with internal prototypes & variables
  2. I have renamed @node_httpAgent into @httpAgent (and same for httpsAgent), because it was more consistent with other class members.

If you are Ok with this, then I'll move forward & propose a patch (likely tomorrow in my timezone)

@asnowfix
Copy link
Contributor Author

Just uploaded #2

@pwnall
Copy link
Owner

pwnall commented Jan 31, 2013

Thank you for #2, I merged it!

How would you feel about a XMLHttpRequest.nodejsSetAgents() method that would set the prototype properties?

This way, you can use the following in your code that uses dropbox.js

Dropbox.Xhr.Request.nodejsSetAgents(....)

The nice thing about this approach is that if dropbox.js ever switches to another XHR library, proxy-setting code will fail right away during development, instead of silently ignoring the agent-related properties.

@asnowfix
Copy link
Contributor Author

In my local copy of Dropbox-js, I rather added a dependency on node-tunnel & gave additional Client instanciation parameter then passed down to xhr2. But I admit that I could rather leaver Dropbox-js un-modified & use the shortcut you propose. (1) Less code is always better than the opposite and (2) that lets dropbox-js of an additional dependency.

I think that the additional interface you propose is fine, but I would rather let you implent it, if you do not mind.

@pwnall
Copy link
Owner

pwnall commented Jan 31, 2013

Sweet, I'll get that in soon, and I will release 0.0.4 once I get some tests for events in.

@pwnall
Copy link
Owner

pwnall commented Feb 3, 2013

I hope this works now!

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

No branches or pull requests

2 participants