-
Notifications
You must be signed in to change notification settings - Fork 35
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
Too much intimacy between this, request, and http.Agent #9
Comments
I would be in favor of a direction that obviates this package rather than modifies it since all these features are going to be in core soon anyway. |
Sure, but I'm mostly concerned that 0.12 is going to bust request and make headaches for both of us.
Hah, obviously I meant "push to npm", not "open source", since Node is all open source already :) |
That's what I figured. I'll adopt it as the default Agent in request once you release it. -Mikeal On May 21, 2013, at 10:39PM, "Isaac Z. Schlueter" [email protected] wrote:
|
Cool, then I will go ahead and make the breaking change. I'm pretty sure that the API in question is only used by you anyway :) |
ForeverAgent has a
addRequest
method, and also anaddRequestnoreuse
method. This overrides the http.Agent'saddRequest
method.I'd really like to change the signature of
agent.addRequest
so that it takes an options object rather than a set of positional arguments. The reason for this is that it would allow us to reuse more of the code between http and https, where there are many more things that separate reusability buckets than just the host and port. You can reuse connections with https, but only if all the following fields are matching: host, port, ca, cert, ciphers, key, passphrase, pfx, and rejectUnauthorized.However, because request and forever-agent use this undocumented API, we can't change it, and it'll break on the upgrade to 0.12. Making things worse, you can't chnage forever-agent's API, because request dives into its guts and sniffs for a
addRequestnoreuse
method.Is there any way to refactor this so that it doesn't rely on internals in this way? What if I open source the new 0.12 http.Agent that has built-in keepAlive if you turn it on?
The text was updated successfully, but these errors were encountered: