yjs / y-websocket

Websocket Connector for Yjs
https://docs.yjs.dev/ecosystem/connection-provider/y-websocket
MIT License
531 stars 262 forks source link

No way to define `path` for WebSocket client #57

Open anotheri opened 3 years ago

anotheri commented 3 years ago

Describe the bug I can't see the way to define the path property for the WebSocket client. When I use URL or room to emulate the path value to match URL, requests go to the server, but the connection never setup as expected. It never passes the handleUpgrade method.

To Reproduce Server:

const wssYjs = new WebSocket.Server({ path: '/yjs', noServer: true});
wssYjs.on( 'connection', ywsUtils.setupWSConnection );

// `app` is Express.js app, `app.server` is https node server here
app.server.on('upgrade', async (request, socket, head) => {
  // async auth is here
  wssYjs.handleUpgrade(request, socket, head, (ws) => {
    console.log('=========\n\n'); // ← THIS NEVER HAPPEN if I define path on websocket server

    wssYjs.emit('connection', ws, request);
  } );
});

and on the client (I tried different combinations but the connection is dropping anyway)

const socketUrl = 'wss://localhost:3000`; // alternative: 'wss://localhost:3000/yjs`
const room = 'yjs/example'; // alternative: `example`
const wsProvider = new WebsocketProvider(socketUrl, room, doc, {
  params: getSocketAuthParams(user)
});

Expected behavior I expect to have a way to define the same path I use on the WS server and that would establish the connection as expected

const wsProvider = new WebsocketProvider(socketUrl, room, doc, {
  path: '/yjs',
  params: getSocketAuthParams(user)
});

Environment Information

Huly®: YJS-513

dmonad commented 3 years ago

Hi @anotheri, I haven't verified but I believe you that this is not working. I currently don't have time to work on this. I'd be happy to receive a PR though.

Just a note: this might be a bug in the wss package.

anotheri commented 3 years ago

this might be a bug in the wss package.

Well, it's possible but I doubt it, b/c I have another wss defined with path: '/ws', which works as expected. But I define path on the client-side as well. So to me, it looks like the only difference between these ones that the client for y-websocket created w/o path property. Also have no time right now to verify this further and prepare PR. :/ But I'll try to prepare PR as soon as I have a chance.

dmonad commented 3 years ago

Cool, thanks :+1: