WebsocketProvider needs to wait to send sync step 1 until after it knows the server is listening

[astnmsn]

Checklist

• Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
• Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/

Describe the bug WebsocketProvider needs to wait to send sync step 1 until after it knows the server has setup its listener (has received sync step 1 from the server)

To Reproduce Steps to reproduce the behavior:

1. Add delay to server between websocket connection and listening to events on the websocket
2. Observe client send sync step 1
3. Message is never processed by the server
4. Server sends sync step 1
5. Client never reaches 'synced' state because the server never responds to the client's sync step 1 (because the server wasn't listening at the time) with the proper next step

Expected behavior Client and server exchange sync step 1 messages, and server initiates the next step after processing the client's first step

Screenshots None

Environment Information

• Browser & Node server
• "y-websocket": "1.3.18"
• "yjs": "13.5.24"

Additional context We are executing some async code as part of the handleUpgrade callback. This is causing delays with setting up the message listener on the websocket connection. There may be another way to handle this, but I found that moving the send of sync step 1 from the client into the handler for messageSync when the message is of type 'messageYjsSyncStep1' solved the issue.

Maybe there is a better way to handle this on the server so it awaits sending the connect to the websocket?

_{Huly®: YJS-515}

[jlvdh]

@a-mason I'm currently running into the same issue. Curious if you could share a bit more into detail how you're currently solving this?

[astnmsn]

@jlvdh Sure. I'll try to get a fork/branch up with the edits in a bit. There may be other ramifications to the change that I haven't uncovered yet. But it seems to be working in my use case.

[astnmsn]

Here is the commit with the change that appears to fix it for me. I haven't been playing around with Awareness too much. So you may need to consider moving the awareness upsync as well (here).

[jlvdh]

Thanks a lot! Just wanted to share with you another solution that seemed to work; putting the async call before the handleUpgrade:

server.on('upgrade', async (request, socket, head) => {
  const parsed = url.parse(request.url, true)
  const token = parsed.query.JWT
  const resourceId = parsed.pathname
  const isAuthorized = await checkAuthorization(token, resourceId)
  await new Promise(resolve => setTimeout(resolve, 3000)); // testing long delay

  const handleAuth = async ws => {
    if (isAuthorized) {
      wss.emit('connection', ws, request)
    } else {
      console.log('access denied')
      setTimeout(() => { 
        console.log('socket destroyed')
        socket.destroy();
        socket = null;
      }, 10000)
    }
  }
  wss.handleUpgrade(request, socket, head, handleAuth)
})

[dmonad]

Thanks for sharing! This is also the solution that is described by the ws library: https://github.com/websockets/ws#client-authentication
It might very well be that this doesn't work if you don't pass an async function.

I added a short hint to the ws docs from the handleUpgrade method. If this still fails, it might make sense to open an issue in the ws project because this doesn't seem to be y-websocket related.

[astnmsn]

We had async code in both in the authentication and as part of the handling of the wss 'connection' event. The authentication was being handled as both you suggested, but it was the latter async code (fetching the doc from persistent storage) that was causing the issue. The reference server handles this by not awaiting the fetch of the document (here). I'm wondering how this would impact the handshake/sync with the client when it goes to write sync step 1? From my understanding the client is expecting the persisted document state in this message. If so, wouldn't this potentially be sending an empty doc if the retrieval from the persistent storage is sufficiently delayed?

[dmonad]

If you wait to register event handlers after you synced with a database you will definitely not receive the sync messages from the client. A solution would be to make sure that event handlers (e.g. onmessage) are registered when the connection is initialized (synchronously).