Increase default keepAliveTimeout to 65s
Summary
This PR increases the default value of http.Server.keepAliveTimeout from 5000ms (5s) to 65000ms (65s). This change aligns Node.js with the keep-alive timeout expectations of most browsers, proxies, and mobile clients, reducing the risk of subtle long-lived connection failures due to premature socket closure.
Details
- Default Change:
ThekeepAliveTimeoutdefault is now set to 65 seconds (65000ms) instead of the previous 5 seconds (5000ms). - Documentation & Comments:
All relevant documentation and code comments have been updated to reflect the new default.
Motivation
The previous default of 5 seconds is shorter than the defaults used by common infrastructure, which can lead to unexpected connection terminations or failures. Setting the default to 65 seconds brings Node.js in line with industry standards and improves interoperability.
References
Please review carefully as this changes default server behavior for all applications that do not explicitly set keepAliveTimeout.
Review requested:
- [ ] @nodejs/http
- [ ] @nodejs/net
@jasnell @targos @lpinca Please review my pull request when you have a chance. Thank you!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.06%. Comparing base (
7215d9b) to head (e59f1f3). Report is 5 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #59203 +/- ##
==========================================
+ Coverage 90.04% 90.06% +0.02%
==========================================
Files 648 648
Lines 191041 191041
Branches 37448 37455 +7
==========================================
+ Hits 172026 172065 +39
+ Misses 11651 11601 -50
- Partials 7364 7375 +11
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/_http_server.js | 97.23% <100.00%> (ø) |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Hey @mcollina ,
Thanks for your feedback on the keep-alive timeout.
I’ve chosen 60 seconds to align with defaults used by major cloud load balancers like AWS ELB/ALB, ensuring broad compatibility and preventing unexpected connection drops. Increasing the timeout to 72 seconds, as seen in Fastify and NGINX, could slightly boost connection reuse, but also raises the risk of file descriptor exhaustion under DoS scenarios.
Overall, 60 seconds provides a safer balance between interoperability and security for most environments. I’m open to discussing further if there’s a strong case for a higher value.
@mcollina , I've resolved this issue and removed the breaking change alert regarding http.Server.keepAliveTimeout. The documentation now reflects the correct status, in line with the release team's guidance. Please review my pull request when you have a chance. Thank you!
I've seen 60 seconds cause problems due to time network delays - I've actually seen this happen. Going over 60 seconds is necessary to account for that.
Okay, sure—that makes sense. Is 72 seconds sufficient to account for those network delays, or do you recommend going even higher? Happy to adjust as needed based on your experience.
I literally picked 72 with no reasoning. We could do 65.
Thanks for clarifying. I’m fine with 65 seconds if that works better—happy to update the default to 65 to provide a bit more buffer over 60. Let me know if you have any concerns or if you think another value might be preferable.
@pras529 there are 4 frailing tests and linting to be fixed, could you address them?
@mcollina Yes, I’ll address the failing tests and linting issues—just need a bit of time to work through them.
Thanks for referencing this and working on a fix!
Looks like a great step forward, and I appreciate you taking the time to address it 🙏
Hello @mcollina,
Thank you for your review. I’ve addressed the changes you requested and updated the commit message to fully comply with our guidelines. I’m still a bit uncertain about the Linux and macOS test results—could you please take a look and let me know if anything else needs adjustment?
Thanks for your help!
I'm on the fence to consider this a breaking change or not. If so, this change will probably go out in v25, but I think it's pretty harmless to ship in v24 as well.
@pimterry wdyt?
@benjamingr any opinion on semversiness?
I think it could be semver minor. This will change some behaviour (more connection reuse, subtly different default response headers, could very mildly increase resource usage in scenarios with many keep-alive-but-no-requests clients) but that's all quite limited, and nothing that would break any reasonable code I can imagine.
The nginx 75s keepalive timeout sounds a lot more reasonable to me
I don't mind going to 75s if people prefer, but I don't think finetuning matters much in practice: the important thing for actual failures (as opposed to peak performance) is the relative difference between the race condition time (client sending a packet before receiving the FIN = roughly RTT, say 5ms) and the total timeout. Node closing before the LB isn't a big problem, as long as the LB isn't sending a request at exactly the same time.
With this change, to hit an error a client needs to send a first request, wait exactly 64.995 seconds, and then send another request just before the FIN comes in. For any likely traffic distribution, I think odds go down exponentially as the timeout goes up, so by 65s this is already extremely unlikely to ever appear in practice. Meanwhile resource usage (no threads for us, but still handles + some memory at least) goes up linearly.
For failures specifically I don't think tuning individual seconds matters much here (for maximising performance in some envs fine tuning definitely does matter, but it's env specific and I'd expect 60s+ should already deliver pretty good results for typical node deployments).
The https://github.com/nodejs/node/labels/notable-change label has been added by @RafaelGSS.
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.
@pras529 can you adjust the first commit message? (You can also squash all commits too)
I'm content with 65 seconds. Appreciate this change greatly.
What's the reason for it be semver-minor and not semver-major? @mcollina
What's the reason for it be semver-minor and not semver-major?
https://github.com/nodejs/node/pull/59203#issuecomment-3124276867
If we think there's a real risk that this could break anything in existing code, or if people would just prefer to be cautious, I'm not opposed to making this major, but personally I'd be quite surprised it caused any issues in practice. For users it should really just be a bug fix & performance improvement.
Do we have any specific likely scenario where this might break things?
It's hard to say its real impact, but by experience, whenever we change a default option on HTTP, it tends to break people. It's also hard to have those actions covered by our test suite.
I feel that we might want to wait for it to land on Node.js 25 and then backport after assessing its impact. I think CITGM wouldn't cover this scenario too.
@RafaelGSSy assumption is that risk in shipping on Node 24 before it goes LTS is close to zero, but very high afterwars.
Hi folks! If you plan to increase keepAliveTimeout to 65 seconds, should you also increase headersTimeout to 66 at least? Please check https://github.com/nodejs/node/issues/27363
A very good question @ex1st! From what I can see though, that issue was resolved by https://github.com/nodejs/node/pull/32329/, which changed how headerTimeout was measured entirely, so it shouldn't overlap with keepAliveTimeout as it used to. From my reading of the code there, nowadays the header timer doesn't start until a new request starts, and it's disabled when it finishes, so it's not measuring idle time at all. Does that make sense?
Is this change still something we're looking to land on Node.js 25? It seemed to get a lot of approvals/support for changing the default at the time.