-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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(ws): close existing connections #13531
Conversation
Existing connections must be closed manually. Otherwise, application shutdown hangs when shutdown hooks are enabled and there are existing connections.
Pull Request Test Coverage Report for Build 7a20893c-f508-44d3-8a50-e1b7ed027b27Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, where will this piece of code will be invoked buddy ?
Hi @benjGam This will be invoked in here: https://github.com/nestjs/nest/blob/master/packages/websockets/socket-module.ts#L108. Since |
Okay, didn't see that, thanks for your reply |
Close all existing websocket connections to ensure a graceful shutdown.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: closes #13534
Existing connections are not closed automatically, according to https://github.com/websockets/ws/blob/master/doc/ws.md#serverclosecallback.
Application shutdown hangs when shutdown hooks are enabled and there are existing websocket connections. This is because the server.close callback that is awaited only resolves after all connections are closed. This is the call chain:
What is the new behavior?
All existing websocket connections are closed to ensure a graceful shutdown.
Does this PR introduce a breaking change?
Other information