nest
nest copied to clipboard
nestjs
fix(ws): close existing connections
Close all existing websocket connections to ensure a graceful shutdown.
PR Checklist
Please check if your PR fulfills the following requirements:
- [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:
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?
- [ ] Yes
- [x] No
Other information
Pull Request Test Coverage Report for Build 7a20893c-f508-44d3-8a50-e1b7ed027b27
Details
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 92.124%
| Totals | |
|---|---|
| Change from base Build e56f6d99-d9bc-426a-b9ef-ed11406d7e1c: | 0.0% |
| Covered Lines: | 6737 |
| Relevant Lines: | 7313 |
💛 - Coveralls
Hi @benjGam
This will be invoked in here: https://github.com/nestjs/nest/blob/master/packages/websockets/socket-module.ts#L108.
Since WsAdapter extends AbstractWsAdapter the AbstractWsAdapter.close method was called before this proposed change.
