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

Is there a chance to use a http/https proxy in proxy.js? #60

Open
CliffHan opened this issue Dec 23, 2015 · 5 comments
Open

Is there a chance to use a http/https proxy in proxy.js? #60

CliffHan opened this issue Dec 23, 2015 · 5 comments

Comments

@CliffHan
Copy link

I checked unblocker/lib/proxy.js, and this module used http and https library to send real request.
It's a little hard to integrate proxy feature.
Maybe you could think about using request library to send request?
So that the proxy feature can be easily integrated.

@nfriedly
Copy link
Owner

nfriedly commented Jan 7, 2016

Hi Cliff, sorry for the delay in responding. Proxy support isn't not something that I think I could implement any time soon. But if you want to put in the work, I'd be interested to see what you come up with and would consider merging it in.

@pepzwee
Copy link

pepzwee commented Oct 5, 2017

If anyone wants to implement request/request module these are the things I did:

  1. Install the module
  2. Require it in lib/proxy.js
  3. Add uri: uri to the options variable
  4. Replace data.remoteRequest = proto.... with
data.remoteRequest = request(options)

data.remoteRequest.on('response', function(remoteResponse) {
    data.remoteResponse = remoteResponse;
    data.remoteRequest.on('error', next);
    proxyResponse(data)
})

Seems to be working with minimal changes to the file. This gives you the option to add a proxy value to the options variable. Should be safe to remove the proto variable as well.

Gist: https://gist.github.com/pepzwee/7dabaec9c3273a94f099e44a4e49e224

@nfriedly
Copy link
Owner

nfriedly commented Oct 5, 2017

I think that

data.remoteRequest.on('error', next);

Should be

data.remoteResponse.on('error', next);

@pepzwee
Copy link

pepzwee commented Oct 5, 2017

Ah, my bad. 😰 Yeah, it should be remoteResponse instead.

@nfriedly
Copy link
Owner

nfriedly commented Oct 5, 2017

Cool. I like keeping the core of the library fairly low-level, but if you want to send a PR to make that part configurable, and then maybe an example that shows how to use request there, I'd be happy to merge that in.

It could be something like a makeRequest configuration option that can be set to a function - have it accept the data object and a callback. It should make the request, update data, then call the callback. Once the callback is fired, then call proxyResponse(data).

And then if makeRequest isn't set, it can default to something pretty similar to the existing code, only with proxyResponse(data) replaced with the callback.

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

3 participants