Fixing socket hang up error on node side for slow response.

[ryouaki]

Sometime, the response will be very slow, and does not respond, the connect event will be block by event loop system.

And timer callback will be fired, and abort() will be invoked before connection, then get "socket hang up" and code ECONNRESET.

At this time, if we have a large number of request, nodejs will hang up some socket on background. and the number will up and up.

And then these socket which be hang up will devoring CPU little by little.ClientRequest.setTimeout will be fired on the specify milliseconds, and can make sure that abort() will be fired after connect.

So I change the code to use the timeout event for connect timeout on nodejs side.

My issue
I think this issue is the same reason

[ryouaki]

@mzabriskie can you help me？

[ryouaki]

@nickuraltsev could you help me?

[RikkiGibson]

It looks like request.setTimeout sets a read timeout handler on the underlying socket. It's not the same thing as a request timeout.

Is the issue is that aborting a request before the connection is established doesn't properly dispose of the socket?

[ryouaki]

@RikkiGibson Yes，abort a request before the connection, and does not release socket.

When the server side is too slow and no responed for a long time, sometimes setTimeout will be invoked before connection, and "socket hang up" will be got. This only appear at connection does not block setTimeout.

I think for Borwser the request timeout is not the same as connection timeout, But on nodejs I think it is the same. and sometimes the connection block setTimeout. So if we set 10s for timeout ,but the connection block the event loop about 20s, the setTimeout will be fire after 20s.

poll: retrieve new I/O events; execute I/O related callbacks (almost all with the exception of close callbacks, the ones scheduled by timers, and setImmediate()); node will block here when appropriate.

[RikkiGibson]

The request timeout applies to receiving the entire HTTP response.

The socket timeout, set by request.setTimeout, applies to any activity on the socket (individual reads and writes). The browser doesn't provide any way to set this.

I said "read timeout" previously because a lot of languages will let you configure read and write timeouts separately, while node appear to combine them into one. Sorry for any confusion.

Let's use the setTimeout function like before so that we don't change the meaning of the timeout property. In the timeout handler, we can do something like the following:

if (request.socket.connecting) {
  request.socket.on('connect', function() {
    request.abort();
  }
} else {
  request.abort();
}

Does that seem reasonable?

[ryouaki]

@RikkiGibson Thank you for your explain,

Maybe my english is not good enough. Yes you want to do timeout like borwser .But sometimes the io event will block setTimeout. the setTimeout will not be invoked in time.

for my case like I said before:

1. I set 10s for timeout by setTimeout function.
2. Send a request to a low server . and the server very busy. and no respond.
3. The connection will block setTimeout function. until connection finish. Nodejs event loop
4. Connection finish after 20s, the setTimeout function will be invoked after 20s.

So, when connection block setTimeout function, The timeout will be fired more than millseconds you set before.

This is my problem.

[ryouaki]

Nobody else?

[ryouaki]

@aguynamedben @Khaledgarbaya

[blackswanny]

I have this issue as well. We have plenty of requests open at the same time

[freitasskeeled]

Hi there,

Is there any updates on this PR? I think I'm suffering from the same problem.

Thanks

[ryouaki]

suffering

I have the same question......

[hangaoke1]

mark

[vivek-kandhvar]

Any update?

[snowdream]

I have the same question

[dalewang1995]

mark

[goulderb]

Is any priority going to be given to this issue? I'm hitting this in production and it's forcing us off axios.

[ryouaki]

Is any priority going to be given to this issue? I'm hitting this in production and it's forcing us off axios.

this pr need another code review