fix: close dead connections even if they have active subscriptions#438
fix: close dead connections even if they have active subscriptions#438tharu-jwd wants to merge 3 commits intocameri:mainfrom
Conversation
|
@tharu-jwd Please address failing ci checks |
There was a problem hiding this comment.
Pull request overview
Fixes a WebSocket heartbeat cleanup bug where dead connections with active subscriptions were never terminated, causing ongoing pings, event matching work, and memory growth.
Changes:
- Remove the
!this.subscriptions.sizeguard so pong timeouts close dead clients even when they have active subscriptions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public onHeartbeat(): void { | ||
| if (!this.alive && !this.subscriptions.size) { | ||
| if (!this.alive) { | ||
| console.error(`web-socket-adapter: pong timeout for client ${this.clientId} (${this.getClientAddress()})`) | ||
| this.client.close() | ||
| return |
|
@tharu-jwd commits must follow the conventional commits spec, please rebase and update your commit messages |
tharu-jwd
force-pushed
the
fix/dead-connections-with-subscriptions
branch
from
679cbc1 to
e1bcfcf
Compare
cameri
changed the title
|
coverage: 74.525% (-0.008%) from 74.533% — tharu-jwd:fix/dead-connections-with-subscriptions into cameri:main |
|
Hi @cameri |
🦋 Changeset detectedLatest commit: b46aa77 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@tharu-jwd yes, please include a changeset file. Use npx changeset to create it. |
tharu-jwd
force-pushed
the
fix/dead-connections-with-subscriptions
branch
from
135a48c to
b46aa77
Compare
Description
The heartbeat was skipping termination for dead clients that had active subscriptions. Removed
!this.subscriptions.sizeguard fromonHeartbeat()as the solution.Related Issue
Fixes #427
Motivation and Context
Dead clients with subscriptions were kept alive forever. Server kept pinging them, matching events against their filters, leaking memory.
How Has This Been Tested?
Manually connected a client, sent a REQ, killed it abruptly, confirmed the server closes it on the next heartbeat cycle instead of continuing to ping.
Screenshots (if appropriate):
N/A
Types of changes
Checklist: