Closed astnmsn closed 2 years ago
@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?
@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.
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).
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)
})
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.
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?
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).
Checklist
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:
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
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