node icon indicating copy to clipboard operation
node copied to clipboard

`http.server.close()` closes idle connections since 18.19.0

Open Llois41 opened this issue 4 months ago • 1 comments

Version

18.19.0

Platform

Linux, Mac, Windows

Subsystem

No response

What steps will reproduce the bug?

import http from 'node:http';
import timers from 'node:timers/promises';

const server = new http.Server((req, res) => res.end());
server.keepAliveTimeout = 5_000;
server.listen(8080);

const agent = new http.Agent({ keepAlive: true, keepAliveMsecs: 60_000 });
await get();
console.log('request 1 completed');

await timers.setTimeout(1_000);

const closed = new Promise((resolve) => server.close(resolve));
console.log('listening socket closed');

await get();
console.log('request 2 completed');

await closed;
console.log('socket completely closed');

function get() {
  return new Promise((resolve, reject) => {
    http.get('http://localhost:8080', { agent }, (res) => {
      res.resume();
      if (res.statusCode !== 200) {
        reject(new Error(`request failed with status code ${res.statusCode}`));
      } else {
        resolve(undefined);
      }
    });
  });
}

Works as expected with Node 18.18.2:

$ node http-close.mjs
request 1 completed
listening socket closed
request 2 completed
socket completely closed

Second request fails on Node 18.19.0 because all idle connections are closed:

$ node http-close.mjs
request 1 completed
listening socket closed
node:events:495
      throw er; // Unhandled 'error' event
      ^

Error: socket hang up
    at connResetException (node:internal/errors:720:14)
    at Socket.socketOnEnd (node:_http_client:525:23)
    at Socket.emit (node:events:529:35)
    at endReadableNT (node:internal/streams/readable:1400:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
Emitted 'error' event on ClientRequest instance at:
    at Socket.socketOnEnd (node:_http_client:525:9)
    at Socket.emit (node:events:529:35)
    at endReadableNT (node:internal/streams/readable:1400:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'ECONNRESET'
}

How often does it reproduce? Is there a required condition?

We upgraded to Node 18.19.0 (from 18.18.2) and saw our test fail that tests graceful shutdown. This tests sends a request before (with keepalive set) and after the http.Server is closed to ensure that all requests will be answered and there will be no HTTP or socket errors.

In the docs it is also explicitly stated that the http.server.close() only stops the server from accepting new connections but not terminating open ones.

What is the expected behavior? Why is that the expected behavior?

The expected behavior ist that calling http.server.close() does not terminate idle connections. Maybe closing (idle) connections should be configurable via an options parameter.

What do you see instead?

http.server.close() calls http.server.closeIdleConnections() which will terminate all idle connections.

Additional information

We think we found the commit that introduced this behavior.

Commit on main: bd7a8087a581557c636525b65a5c0e0db58735e7 (only extracts a function)

Commit on tag 18.19.0: 29697229b61a8cbdf12e6abe0cbaa9fecc9a5102 (adds the .closeIdleConnections() call)

Llois41 avatar Feb 06 '24 09:02 Llois41

The commit in main branch is correct, but the backport is not. https://github.com/nodejs/node/pull/50194

climba03003 avatar Feb 06 '24 09:02 climba03003

seems like it's same as. https://github.com/nodejs/node/issues/52330

kumarrishav avatar Apr 02 '24 18:04 kumarrishav

CC: @bnoordhuis

kumarrishav avatar Apr 02 '24 18:04 kumarrishav

Awesome, thanks for the fix that will presumably be released with the next Node 18 version. :tada:

Does anyone know if/how one can implement a robust graceful shutdown with Node 19+? Right now we're doing something like:

const server = new http.Server(...);
server.closeIdleConnections = () => undefined;

to avoid server.close() closing idle connections and we're looking for something more robust/supported/documented.

michael42 avatar Apr 26 '24 12:04 michael42