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

Sync issues Safari ⇔ Chrome #19

Open
dmonad opened this issue Nov 7, 2020 · 6 comments
Open

Sync issues Safari ⇔ Chrome #19

dmonad opened this issue Nov 7, 2020 · 6 comments
Assignees
Labels

Comments

@dmonad
Copy link
Member

dmonad commented Nov 7, 2020

This is a great issue for people who wish to contribute #helpwanted

Describe the bug
Chrome and Safari don't sync

To Reproduce
See issue description: https://discuss.yjs.dev/t/problem-with-webrtcprovider-connect-after-disconnect/278

Expected behavior
Safari keeps up with Chrome

Environment Information

  • Unknown

Additional context
This is probably an issue internal to simple-peer - the webrtc module that is used in y-webrtc.

@dmonad dmonad added bug help wanted first issue A good first issue labels Nov 7, 2020
@dmonad dmonad self-assigned this Nov 7, 2020
@dmonad
Copy link
Member Author

dmonad commented Nov 7, 2020

A good first start would be to reproduce the issue and check the logs. This issue might also get fixed by simply updating simple-peer.

@froger
Copy link

froger commented Jan 21, 2021

Just going through this issue, it seems Safari might have issues with their Webrtc implementations.
(feross/simple-peer#672 (comment), https://bugs.webkit.org/show_bug.cgi?id=198545).

Will try a bunch of safari version as soon as possible.

@snqb
Copy link

snqb commented Feb 6, 2023

hey, have there been any news on this?
I noticed this behavior while syncing my app data from my laptop(Windows with Brave) to my phone(iOS with Safari)
it does sync fine when you just boot it up, but after some time the connection is lost
and the only remedy is to restart the app

@jeffrafter
Copy link

jeffrafter commented Aug 23, 2023

I was having a similar issue. I replaced the y-webrtc implementation with a stub based on @disarticulate's fork (see also #23) and was able to debug more easily.

One of the things I was seeing was:

Failed to execute 'send' on 'RTCDataChannel': RTCDataChannel.readyState is not 'open'

This was happening fairly consistently when a Safari based client would attempt to join a room with Chrome-based clients.

I looked around for connected issues:

And these helped. Though I haven't fully solved it, I'll keep some notes here: something about the order/state between the two browsers is inconsistent. Chrome <-> Chrome is working well Safari <-> Safari is working well. I tried setting filterBcConns: false when creating the provider to see if it was a broadcast channel problem (this did not help). I removed all crypto (this did not help). I updated simple-peer and Int64 deps (from @disarticulate's version, this did not help).

I'll note that I am dealing with very small documents - chunking was not the problem.

I tried adding two additional checks for this.connected:

    this.peer.on("error", (err) => {
      log("Error in connection to ", logging.BOLD, remotePeerId, ": ", err)
      // THIS LINE IS IMPORTANT WHEN RECONNECTING
      if (!this.connected) return
      announceSignalingInfo(room)
    })

And

    this.peer.on("data", async (data) => {
      const valid = await validMessage(room, data.slice())
      if (!valid)
        return console.warn("!InvalidData", {
          peer: this.peer,
          context: this,
          room,
          data,
        })
      const answer = readPeerMessage(this, data)
      //JEFF: THIS LINE IS IMPORTANT WHEN SETTING UP CONNECTIONS
      if (!this.connected) return
      if (answer !== null) {
        sendWebrtcConn(this, answer)
      }
      return
    })

Which in y-webrtc is here: https://github.com/yjs/y-webrtc/blob/master/src/y-webrtc.js#L228-L237

This fixed almost everything. I also have a heavily offline app that refetches on focus and this really made it feel solid.

The only thing this isn't solving for me is when the browser is offline (not connected to a network provider, navigator.onLine is false... faking it in the Network panel is not enough). In that case the browsers go back to not being able to talk to each other. I see more invalid message errors so an additional check might be needed.

Some conclusions: sending a message when not connected properly crashes the Peer. It goes away entirely and you have to refresh to fix it. But you have to do this to all of the peers and build it up again. If you have an application that refreshes on focus (especially with HMR) this gets very difficult to debug.

I'm interested to know if the this.connected checks feel right. If so I'm happy to open a PR.

lirsacc added a commit to lirsacc/siren-chorus that referenced this issue Nov 27, 2023
I'd prefer WebRTC over websocket here (especially as the WS
implementation isn't encrypted; however I've run into issues around
cross browser collaboration - namely:

yjs/y-webrtc#56
yjs/y-webrtc#53
yjs/y-webrtc#19

For now having websocket allows iterating on the editor and the UX until
I can look at those issues more in depth.
lirsacc added a commit to lirsacc/siren-chorus that referenced this issue Nov 28, 2023
I'd prefer WebRTC over websocket here (especially as the WS
implementation isn't encrypted; however I've run into issues around
cross browser collaboration - namely:

yjs/y-webrtc#56
yjs/y-webrtc#53
yjs/y-webrtc#19

For now having websocket allows iterating on the editor and the UX until
I can look at those issues more in depth.
@tim-hilt
Copy link

tim-hilt commented Dec 21, 2023

Hey, are you sure this issue is limited to Chrome and Safari? I couldn' t get cross browser syncing to work at all. I tested with Safari, Arc and Chrome.

This issue might be related to #63 and #53

@navaneeth-automatad
Copy link

Hey, any progress in this issue ?

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

6 participants