Added proxy support

[jsebfranck]

Hi,

To resolve the issue in contentful.js lib : contentful/contentful.js#16. I implemented the proxy support in questor. Please note that it worked only for http requests and not for https requests.

As soon as you will accept this PR, I will do another PR in contentful.js library.

Thanks.

[seidtgeist]

Thank you. Would it also be possible to achieve the same by passing in those "proxified" options from outside, or am I missing something?

[jsebfranck]

Hmmm I'm not sure to understand what do you mean...

With this PR, we can pass these options through options parameter in questor main method. When you will accept the PR, I will modify Contentful.js in order to put the proxy options from Contentful.js options to questor. Does it make sense for you?

[seidtgeist]

I'd be willing to change the interface so you can pass options or a URI. That way all the proxy options could be passed in from the outside. This would be a major bump of course.

The thing is I'm not sure I want to have explicit proxy support in questor, because it's something that's not going to work in browsers. The whole point of questor was to have a very thin but mostly consistent layer for doing HTTP requests in node and in browsers. Adding features that are only supported on one of the platforms breaks that promise.

What do you think?

[sdepold]

I would be fine with keeping the API the way it is. Which change would break browser land?

[jsebfranck]

I agree that it wouldn't break the browser but I also understand @ehd vision of questor responsability.

What could I do now is to pass a method in questor options and to call it if it is defined. What about that? For example :

if (options.modifyRequest) {
  options.modifyRequest(uri, options);
}

Then in contentful.js, I can pass initProxy method in questor modifyRequest method :

options.modifyRequest = initProxy;

What is sad however is that we will have to do the same in all Javascript questor clients : contentful-management.js, contentful-publication......

[sdepold]

I still don't get what the problem with this PR is. It looks fine for me. The api is sane and the browser shouldn't have a problem with it, should it?

[jsebfranck]

@ehd?

[seidtgeist]

@sdepold The problem is that the API suggest that you can influence proxy behaviour for browser clients, which isn't possible. It would be misleading to have this option as part of the API of a "hybrid http request client" when it doesn't do anything for one of the two platforms it supports.

@jsebfranck That goes into the direction I'm more comfortable with. One of these two options:

1. Pass in all parameters, including proxy arguments
2. Provide a request mapping function that modifies the request, but I think (1) makes more sense where you just pass the already modified request object in.

What is sad however is that we will have to do the same in all Javascript questor clients : contentful-management.js, contentful-publication......

You should create a proxify-questor-options package that transforms a normal request + proxy arguments into a proxified request object. I'm still willing to change questor's API to accept an options object which also includes the parsed URI.

[sdepold]

I see! OK. So I'd be fine with changing the API to having only a single param which gets piped into the request call. Afterwards a major version bump and everyone should be happy. Let's please update the readme accordingly.

[jsebfranck]

Finally I'm a little bit lost in this conversation. What do you want to do exactly? In the meantime, we are using my fork in production.