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: don't subscribe to notifications in sk connections #1736

Merged
merged 1 commit into from
May 16, 2024

Conversation

tkurki
Copy link
Member

@tkurki tkurki commented May 16, 2024

@signalk/client subscribes by default to notifications.*. This is confusing in the first place if the user enters an explicit subscription and gets more than they subscribed.

Furthermore this wreaks havoc if you have two interconnected SK servers. Their explicit subscriptions will not create a loop if they don't touch the same paths, but the default notifications subscription will if there is even one active notification.

@signalk/client subscribes by default to notifications.*.
This is confusing in the first place if the user enters an
explicit subscription and gets more than they subscribed.

Furthermore this wreaks havoc if you have two interconnected
SK servers. Their explicit subscriptions will not create a
loop if they don't touch the same paths, but the default
notifications subscription will if there is even one
active notification.
@tkurki tkurki added the fix label May 16, 2024
@tkurki tkurki merged commit 4748c17 into master May 16, 2024
4 of 5 checks passed
@tkurki tkurki deleted the fix-dont-subscribe-sk-notifications branch May 16, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant