Skip to content
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

fix: close idle connections when stopping FastifyController #857

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Sep 23, 2024

In Node 18, calling HTTP.Server.close() does not close idle connections, which resulted in some undesirable behavior in the server lifecycle in CoMapeo Mobile (see digidem/comapeo-mobile#748 for more context). In Node >=19, closing the server does the equivalent work of of closeIdleConnections() internally.

This PR explicitly calls closeIdleConnections() when stopping the server, at the guidance of the Node docs:

Starting with Node.js 19.0.0, there's no need for calling this method in conjunction with server.close to reap keep-alive connections. Using it won't cause any harm though, and it can be useful to ensure backwards compatibility for libraries and applications that need to support versions older than 19.0.0. Whenever using this in conjunction with server.close, calling this after server.close is recommended as to avoid race conditions where new connections are created between a call to this and a call to server.close.

There's probably an easy way to test this but wasn't immediately sure of how to do it. Happy to add one with some guidance.

@achou11 achou11 merged commit d4b9904 into main Sep 23, 2024
6 checks passed
@achou11 achou11 deleted the ac/fastify-controller-close-idle-connections branch September 23, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants