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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typing status is not relayed by version 1.2.4 of the external signaling server #12252

Closed
danxuliu opened this issue May 3, 2024 · 3 comments
Closed
Labels

Comments

@danxuliu
Copy link
Member

danxuliu commented May 3, 2024

How to use GitHub

  • Please use the 馃憤 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Since version 1.2.4 of the external signaling server the roomType parameter in signaling messages must be one of the predefined values (audio, video or screen). If the room type is not valid then the message is discarded without relaying it to the other participants, and an error message is logged; for example:

hub.go:1450: Invalid message {"type":"message","message":{"recipient":{"type":"session","sessionid":"XXX..."},"data":{"type":"startedTyping","to":"XXX..."}}} from client YYY...: invalid room type:

Signaling messages are used to send the startedTyping and stoppedTyping messages to other participants, either when the typing status changes or when a participant joins a conversation. As those signaling messages are unrelated to calls no roomType was included, so now those messages fail with the external signaling server 1.2.4.

I am not sure if it would be better to just set the roomType to video even if it does not really fit (like already done for signaling messages in calls when there is no peer or when the message refers to the whole participant, like reactions or raised hands), or change the external signaling server code to allow again messages without room types, add another room type for messages unrelated to actual signaling between call peers, or limit the room type validation to messages related to actual signaling between call peers.

@fancycode What do you think? Thanks!

@danxuliu danxuliu added 1. to develop bug regression feature: signaling 馃摱 Internal and external signaling backends labels May 3, 2024
@nickvergessen
Copy link
Member

change the external signaling server code to allow again messages without room types

Or to simply list all types we use?
Helps with documentation and makes it clearer.
Should also not use video for call reactions? Sounds like bug abuse

@fancycode
Copy link
Member

As stated in the past, I would prefer you use the transient data feature for the typing indication ;-)

Anyway, the problem comes from the fact that the JSON unmarshalling in
https://github.com/strukturag/nextcloud-spreed-signaling/blob/94a8f0f02b45a419acf152c59a5acf3c42166925/hub.go#L1622

doesn't trigger an error even though you clearly don't send a MessageClientMessageData as defined here:
https://github.com/strukturag/nextcloud-spreed-signaling/blob/94a8f0f02b45a419acf152c59a5acf3c42166925/api_signaling.go#L560-L566

Only the type is included in your object, so I will either relax the validation to also allow empty roomType values (which you are not sending) or handle incomplete MessageClientMessageData where required fields are missing the same as unmarshalling errors.

Could you please open an issue in the strukturag/nextcloud-spreed-signaling repository? Thanks.

@danxuliu
Copy link
Member Author

Fixed in next nextcloud-spreed-signaling version, see strukturag/nextcloud-spreed-signaling#733

Thanks a lot @fancycode !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants