Created by: glasser
Avoids unhandleable ECONNRESETs.
I think this is arguably a Node http bug (in 0.10.18 at least), but: if you run
http.request({headers: {connection: 'keep-alive'}, agent: false});
During the ClientRequest
constructor, self.shouldKeepAlive
is set to false because there is no agent. But then it calls (indirectly) the storeHeader
function (in http.js
, which sets self.shouldKeepAlive
to true because you specified the keep-alive header.
Then once the request is over responseOnEnd
(in http.js
) runs. Because shouldKeepAlive
is true, we do NOT destroy the underlying socket. But we do remove its error listener. However, because we do NOT have an agent, nobody cares that we emit free
, and the socket is essentially leaked.
That is, we continue to have an open socket to the target server, which has no error
listener and which will never be cleaned up.
It's bad enough that this is a resource leak. But to make matters worse: let's say that the target server dies. This socket will emit an ECONNRESET error... and since there is no error
listener on the socket (there's a listener on the ClientRequest
! but not on the socket), bam, time for an incredibly confusing error to be thrown from the top level, probably crashing your process or triggering uncaughtException
.
I think the best workaround here is to ensure that if we have no agent, then we don't send connection: keep-alive. This PR is one implementation of this.
(I'll work around this a different way in Meteor for now: by providing an explicit Agent.)