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

No way to define path for WebSocket client #57

Open
anotheri opened this issue Mar 12, 2021 — with Huly for GitHub · 3 comments
Open

No way to define path for WebSocket client #57

anotheri opened this issue Mar 12, 2021 — with Huly for GitHub · 3 comments
Assignees
Labels
bug Something isn't working

Comments

Copy link

anotheri commented Mar 12, 2021

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

  • Chrome
  • yjs@13.4.14
  • y-websocket@1.3.10

Huly®: YJS-513

@anotheri anotheri added the bug Something isn't working label Mar 12, 2021
@dmonad
Copy link
Member

dmonad commented Mar 12, 2021

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
Copy link
Author

anotheri commented Mar 12, 2021

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
Copy link
Member

dmonad commented Mar 12, 2021

Cool, thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants