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

No messages sent to guests by the external signaling server when rooms are updated #2146

Open
danxuliu opened this issue Sep 2, 2019 · 3 comments
Labels
1. to develop bug feature: signaling 📶 Internal and external signaling backends low
Milestone

Comments

@danxuliu
Copy link
Member

danxuliu commented Sep 2, 2019

If I am not mistaken this needs to be fixed both in Talk and in the external signaling server; in Talk the update messages should include the session IDs of the guests and in the external signaling server those IDs should be used to send the same messages currently sent to users.

Another option could be that the update messages include the full list of properties besides the name and type; update messages trigger Room.UpdateProperties in the external signaling server, which would send room messages to the clients when the known properties are updated (but this would require to change the external signaling server code whenever a new updateable property is added). This is why currently changing the room name sends a room message to guests (which triggers a synchronization that updates the UI) even if BackendNotifier::roomModified does not provide the guest sessions, but no messages are sent when other properties change.

@fancycode

How to test

  • Setup the external signaling server
  • Create a public conversation as a user
  • Copy the conversation link
  • In a private window, open the copied link
  • Back to the user, enable the conversation for moderators only
  • Change again to the private window

Expected result

The lobby is immediately shown in the private window

Actual result

The lobby is not immediately shown in the private window; eventually it will be shown, but only due to the periodic forced synchronization. You can comment that line to see the lack of updates more clearly.

@danxuliu danxuliu added bug feature: signaling 📶 Internal and external signaling backends labels Sep 2, 2019
@danxuliu danxuliu added this to the 💚 Next Major (18) milestone Sep 2, 2019
@danxuliu danxuliu mentioned this issue Sep 2, 2019
6 tasks
@fancycode
Copy link
Member

@danxuliu we were discussing this yesterday at the conference. The reason for the missing notification in this case is that the properties being sent to the signaling server don't change - so there is no room event being sent to the users and guests.

What is being sent is the roomlist update event to users, but this is intentionally not sent to guests as they don't have a room list.

The solution is to add the new property to the notification code in BackendNotifier::roomModified.

@wiswedel
Copy link
Contributor

Could this also have the effect that some guests no longer receive chat messages? Just had that in a call where I was guest among others and some saw chat messages from the (registered) host and some didn't.
If this is not related, I'm of course gonna open another issue.

@nickvergessen
Copy link
Member

Chat is unrelated to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop bug feature: signaling 📶 Internal and external signaling backends low
Projects
None yet
Development

No branches or pull requests

4 participants