conditionally destroy

[loren138]

What It Does

Conditionally destroy

How To Test

n/a

[loren138]

The reasons for the code are best explained http-party#813 and http-party#966

I think I've finally sorted out what happens in my head.

1. Client aborts the connection (which destroys the socket on the request)
2. That also closes the response
3. res.on('close') fires
4. That destroys the proxyRequest
5. Destroying the proxyRequest results in an econnreset error
6. That triggers the error handler
7. The error handler looks at the originally request to see if it was destroyed (which I think means canceled), if it was it emits econnreset rather than error.

Alternatively, if the proxyReq/response was destroyed meaning the server we are proxying too aborts/crashes/bails (which is what happens in it('should proxy the request and handle timeout error (proxyTimeout)', function(done) {
Then, the clients req.socket is still fine (it has not been destroyed), so we emit an error.

Based on all of that, I think we should be logging debug or maybe info for econnreset in node-api-gateway.

Because of the metrics, we may want to check if req.socket.detroyed in onProxyError rather than ignoring the event entirely.

[loren138]

I'm still undecided if the extra guard added in this PR is helpful or not.
I think res is either closed because we're done in which case the proxyReq is also done anyway (so I think the destroy is a noop) or it's closed because of the abort in which case we should also abort.

I guess the question is, is it more likely that the "extra" .destroy() is a problem in the future or that .writableEnded becomes true also if the port was destroyed so we go back to the memory leak state.

Risks both ways, I lean towards leave out the guard and not merging this, but I'm willing to merge it. @asivakumarbnr thoughts?