https

[srossross]

• Pushed the options logic for targets and agents to from caronte.createProxyServer to common.setupOutgoing
• Added headers to options. Can now overwrite headers (see https.js example 2)
• Added https example. please review the example as I am not sure if this is the intended usage

Also you will notice that the third option raises an error and exits the node interpreter. I am not sure where this error is coming from or where it can be caught.

[coveralls]

Coverage decreased (-0.62%) when pulling 4ee96dd on srossross:caronte into 69f126b on nodejitsu:caronte.

[yawnt]

hey, thanks for the PR

i'm okay with the idea of allowing overridable headers, not so much about the agent..

as you might have noticed we disabled it by default and i don't see why would someone want to specify a different agent for each connection (if you can bring a valid usecase i'd be happy to reconsider my position :D )

the philosophy behind caronte is to keep everything as lean and simple as possible.. overloading common.setupOutgoing with features means, ultimately, slowing down proxy.web and proxy.ws .. that's why i don't want to introduce anything new unless critical.. it would also be really good that every modification to caronte's behaviour was coupled with a test case

last thing.. i don't want to sound too picky but i think that keeping a consistent style is important.. please use /* */ for multiline comments and no comma-first.. (i'm in the process of writing a CONTRIBUTING.md, should be available in the next few days)

[srossross]

@yawnt thanks for the review. I opened this PR as a discussion. I am happy to follow your CONTRIBUTING.md guidelines and add tests.

As for the use case, my goal was to make the options object consistent between the global caronte.createProxyServer and the per request proxyServer.web|ws calls. What do you think?

Let me know if you think this it is the right direction and I will modify and add tests.

[yawnt]

i see what you mean.. i think the global options should be used for things that are not subject to changes often..

how about we settle for removing agent from options.target/options.forward.. then we can set the agent in common.setupOutgoing using outgoing.agent = options.agent || false (it should default to false if none is specified).. we should remove the default agent new Agent... and leave that to the user.. that means also taking out maxSock and adjusting documentation/lib/caronte.js

if we do this you can then specify a global Agent and override it locally for free (since extend is used)

about the tests please look at the way lib-common-test.js is setup and follow the same path.. if you have questions, ask ahead

[coveralls]

Coverage decreased (-0.1%) when pulling 1b5fb1d on srossross:caronte into 69f126b on nodejitsu:caronte.

[coveralls]

Coverage decreased (-0.1%) when pulling a350fad on srossross:caronte into 69f126b on nodejitsu:caronte.

[coveralls]

Coverage remained the same when pulling 39b0c46 on srossross:caronte into 69f126b on nodejitsu:caronte.

[yawnt]

when you feel like it's done give me a shout and i will review :) .. lookin good so far.. only thing i'd change is.. in README.md instead of [0] and [1] write the actual links [ my text ]( my link )

[coveralls]

Coverage increased (+0.64%) when pulling 7ad5c0f on srossross:caronte into 69f126b on nodejitsu:caronte.

[srossross]

@yawnt let me know what you need to pull this in.

[yawnt]

👍 perfect :)

[yawnt]

argh, i just saw you mentioned me in a commit about this being done.. i don't remember seeing it.. i apologize for the late response :(