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

webrtc does not sync well sometimes when there are images #12

Closed
2 tasks done
Marius-Romanus opened this issue May 11, 2022 · 4 comments
Closed
2 tasks done

webrtc does not sync well sometimes when there are images #12

Marius-Romanus opened this issue May 11, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Marius-Romanus
Copy link

Checklist

Describe the bug
Hello, when a second user connects to the webrtc in a quill with an image inserted, quill is not updated to what is in the webrtc, it goes blank. It doesn't always do it... although in Firefox I have gotten these two error messages, in case it can help:

image

It only happens when the second user enters the webrtc room, if both users are already inside, it synchronizes well.
I'm in an angular environment and using ngx-quill though I don't think that has anything to do with it.

I don't know very well if it has to do with the base 64 size of the image or something like that... I have detected that when it fails and doesn't update the content it tries to make several subscriptions.

Captura de pantalla 2022-05-11 202358
...
Captura de pantalla 2022-05-11 202446

On the other hand, I have also seen that when you put a password on the webrtc, it sends a first empty subscription attempt and then with the correct room. I don't know if this has to be like this since I'm new to webrtc.

And finally a third thing I've seen, although I don't think it's related to quill, it seems to do a provider.disconnect(); you are not throwing the websocket (in webrtc) from unsubscribe to the server.

I'm sorry if I don't explain myself very well in English, ask me anything you need, I'll be happy to help, you do a great job.

To Reproduce
Steps to reproduce the behavior:

  1. Create an example quill with webrtc.
  2. Create the webrtc server, (I have used the example you have https://github.com/yjs/y-webrtc/blob/master/bin/server.js)
  3. That a user connect to the webrtc and insert an image.
  4. Connect with a second user.
  5. I don't know if you'll be able to reproduce it... it seems like something very specific and I haven't really found what's going on... I only know what happens when there are images...

Expected behavior

  1. The synchronization should be correct for the second user
  2. Shouldn't I throw an empty subscription websocket when the webrtc has a password? (not sure about this)
  3. disconnect() should launch an unsubscribe websocket to remove it from topics

Environment Information

  • Chrome 101.0.4951.64 (Build oficial) (64 bits)
  • Firefox 100.0 (64-bit)
  • Node: v16.14.0
  • ws: 8.6.0
  • y-protocols: 1.0.5
  • y-quill: 0.1.5
  • y-webrtc: 10.2.3
  • yjs: 13.5.36

greetings and thanks

@Marius-Romanus Marius-Romanus added the bug Something isn't working label May 11, 2022
@dmonad
Copy link
Member

dmonad commented May 12, 2022

Hi @Marius-Romanus,

I believe this issue occurs because some WebRTC implementations have a limit on the maximum message-size. So if you send something like images via WebRTC, then the message might not arrive fully on the other side. This is why you get a out-of-bounds error.

The following PR attempts to fix that. yjs/y-webrtc#25

Maybe you can switch to their implementation and try out if it fixes the issue.

@Marius-Romanus
Copy link
Author

Hello, @dmonad I have been able to verify that with the branch: https://github.com/yjs/y-webrtc/tree/fd6f29b6e692a23c7fe5034cc4eee55af98ea55c, which you indicate, it works and no longer gives that error, but on occasion it has been left in a loop launching a warning that did not contain the type of error it was giving.

image

image

However, I don't know if it would be correct to use it in a production environment since it is not kept up to date, a pity that the polyfill was not implemented at the moment. With your experience, do you think it is better at the moment to use websockets for large sizes instead of webrtc? Thank you very much, I hope that at least my test will serve for you to continue on that path :)

The other two errors, I imagine, are totally independent of this. (First empty subscription when webrtc has a password and disconnect doesn't send unsubcribe)

Greetings!

@dmonad
Copy link
Member

dmonad commented May 14, 2022

Yeah, it might be better to revert to y-websocket for a serious application. Unfortunately, there are still compatibility issues between browsers when using webrtc-data channels.

@dmonad
Copy link
Member

dmonad commented May 14, 2022

Regarding the signaling issue. I think one of my signaling servers is down at the moment. This might be the reason.

@dmonad dmonad closed this as completed May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants