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: only attempt to subscribe/unsubscribe to channels when socket id is set #6

Conversation

nicobritos
Copy link
Contributor

  • Only attempt to subscribe to a private channel if socket id is set, as it is necessary for authenticating a private channel on the server side.

Copy link
Owner

@kerimamansaryyev kerimamansaryyev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done, @nicobritos. Thank you very much!

@kerimamansaryyev
Copy link
Owner

I think, the same should be done on Public Channels

@kerimamansaryyev kerimamansaryyev changed the title fix: only attempt to subscribe to private channels when socket id is set fix: only attempt to subscribe/unsubscribe to channels when socket id is set Aug 3, 2022
@kerimamansaryyev
Copy link
Owner

Checking if [connectionDelegate.socketId] is set on both subscription and unsubscription of both type of Channels

@kerimamansaryyev
Copy link
Owner

@nicobritos , Please have a look at the latest changes to the branch. If it's ok, I will merge it to master.

@nicobritos
Copy link
Contributor Author

@Mcfugger yeah, I think it's OK. I didn't notice but unsubscribing a channel when the socket id is null (therefore, the connection has been closed) makes no sense, so I think it ok.

@kerimamansaryyev
Copy link
Owner

@Mcfugger yeah, I think it's OK. I didn't notice but unsubscribing a channel when the socket id is null (therefore, the connection has been closed) makes no sense, so I think it ok.

Actually, it does make sense - it may be waste calls to the server even it ignores them

@kerimamansaryyev kerimamansaryyev merged commit f753392 into kerimamansaryyev:master Aug 3, 2022
@nicobritos nicobritos deleted the fix-subscribing-without-socketid branch August 4, 2022 13:07
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