-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
http, net: socket "inactivity" timeout #3319
Comments
Please correct me if I'm wrong, but doesn't net use |
I have the same issue, a long running post request is killed by the timeout even though there is some activity. |
This sounds like a code bug rather than a doc bug. Edit: Is this the default 120 seconds found at /lib/_http_server.js:243? I think I've unknowingly run into this issue before. I wrote a small helper script that bugged out on a couple files the first time, but, with no change, worked again the second time. Couple months ago, haven't used it since tho |
@tflanagan yes, 120 seconds defined here. You can also do it on a per-request basis with I'm not sure if an implementation that keeps sockets alive on activity isn't a DoS risk, so I marked this a docs issue. |
It certainly is a DoS risk. However, an unconditional timeout is one extreme, whereas the subsequent documentation on setting it to Perhaps a new |
Hi, I was also surprised by this. I was used to a socket inactivity timeout since long time with nodejs request.setTimeout but it is no more, it's now a global timeout. Should we fix the code or the docs so? |
Closing in favor of #5899 which has a bit more detail and a example. |
d258fb0 added this explanation to
server.timeout
:I noticed that long running (but active) requests like file uploads were cancelled after the timeout had passed and I'm almost certain that there is no actual code that checks for activity on a socket.
Should we update the docs so they state that the socket timeout is actually unconditional? Alternatively, we could possibly reset the timeout on activity, but that will have a perf impact.
The text was updated successfully, but these errors were encountered: