search

yjs / y-quill

Quill Editor binding for Yjs
https://demos.yjs.dev/quill/quill.html
MIT License
80 stars 18 forks source link

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

Closed Marius-Romanus closed 2 years ago

Marius-Romanus commented 2 years ago

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

greetings and thanks

dmonad commented 2 years ago

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. https://github.com/yjs/y-webrtc/pull/25

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

Marius-Romanus commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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