Skip to content

Conversation

@kaiburjack
Copy link

@kaiburjack kaiburjack commented Nov 7, 2025

This is an initial draft implementation for #60617 .
Please see that issue for a detailed discussion of the proposed enhancement.

The implemented behaviour is:

  • upon http.Server.close() or http.Server.closeIdleConnections() we set a new variable drainStarted to true
  • then, via a wrapper for the provided requestListener in the Server constructor, for any then received requests, we set the Connection: close response header, leading to the client to close the connection itself (avoiding the race condition mentioned in Support draining of keep-alive connections on http.Server.close() #60617 )
  • then, in http.Server.closeIdleConnections() we wait (via setTimeout) for the configured drainTimeout before doing anything with idle connections
  • after the timeout callback fires, we proceed normally with what closeIdleConnections() would have done earlier (actually closing any remaining idle connections)

The default behaviour of immediately closing idle connections on "graceful" server shutdown is preserved with the default of dranTimeout == 0.

I am aware of the fact that currently, missing are (as per committer guidelines):

  • documentation changes
  • tests when drainTimeout > 0

But I'd like to first gauge the possibility of this feature actually making it through, eventually.

Currently, all existing tests (via make test) pass successfully and using the patched node executable, the following results in the expected behaviour of the server still accepting requests on idle connections for 5s and responding to them with Connection: close before finally shutting down:

var http = require('http');
var server = http.createServer({drainTimeout: 5000}, function (req, res) {
  res.writeHead(200, {'Content-Type': 'text/plain'});
  res.end('');
}).listen(3000);
function gracefulShutdown(sig) {
  console.log(`${sig} signal received. Draining connections...`);
  server.close(() => {
    console.log('Closed remaining connections');
  })
}
process.on('SIGTERM', () => {
  gracefulShutdown('SIGTERM')
});
process.on('SIGINT', () => {
  gracefulShutdown('SIGINT')
});

What's next

  • Awaiting general acceptance/rejection of such a feature enhancement
  • Improving the drain process (as also mentioned in Support draining of keep-alive connections on http.Server.close() #60617 ) by being more "reactive" to the client actively closing off connections (either because of its own idle timeout expiring or because of a request which the server now responds to with Connection: close) until the number of open connections reaches zero; instead of waiting the full drainTimeout (EDIT: actually, I added that now)

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants