search

sockjs / sockjs-node

WebSocket emulation - Node.js server
http://sockjs.org
MIT License
2.09k stars 309 forks source link

Allow raw websocket connections to the base prefix. #281

Closed jameshilliard closed 3 years ago

jameshilliard commented 3 years ago

This allows accepting websocket connections from non-sockjs clients without having to append /websocket to the url.

This is handy if one wants/requires a specific websocket url endpoint without a /websocket suffix for non-sockjs clients.

This should be fully backwards compatible as websocket requests to the url prefix fail currently.

brycekahle commented 3 years ago

Is there a reason to not use the /websocket suffix, other than personal preference?

jameshilliard commented 3 years ago

Is there a reason to not use the /websocket suffix, other than personal preference?

It's mostly to simplify client interoperability, this way one doesn't have to modify an existing websocket client endpoint when adding sockjs fallback support. Essentially this allows sockjs to function as a drop in replacement for both raw websocket and sockjs clients.

brycekahle commented 3 years ago

Thanks for the PR, but I don't think this additional code/logic is worth supporting what boils down to personal preference. SockJS already handles falling back from true WebSockets to other transports.

jameshilliard commented 3 years ago

supporting what boils down to personal preference

It's less that and more so that one can use this with existing non-sockjs clients as is. Say for example if I need to emulate a particular websocket path expected by a client, is there another way to do that?

SockJS already handles falling back from true WebSockets to other transports.

This was for cross compatibility with non-sockjs clients.

brycekahle commented 3 years ago

You can use non-SockJS WebSocket clients by adding /websocket to the path. You don't want to do that. That is personal preference.

is there another way to do that?

I don't know what web server you are using, but you may be able to accomplish what you want by adding a route for the expected path after SockJS, detecting if it is a WebSocket, and then re-routing to the /websocket route.

It was never the intention of this library to support simultaneous use with a vanilla WebSocket. The idea is drop in SockJS where you were using a WebSocket, which it can do. It will attempt to use WebSockets first, so you don't lose its benefits.

Adding additional code and complexity to maintain for non-SockJS clients, is not something I'm interested in. SockJS servers expose a particular API that is tested for and expected by the clients. If you want something custom on top of that, it is your responsibility to figure out to accomplish that. There are a million ways someone may want to customize, which is not the purview of this library.

jameshilliard commented 3 years ago

I don't know what web server you are using, but you may be able to accomplish what you want by adding a route for the expected path after SockJS, detecting if it is a WebSocket, and then re-routing to the /websocket route.

Well I'm using sockjs-node with express, so I suppose I could probably just bind the sockjs raw websocket route handler to the express route path that's expected by non-sockjs clients, although not sure if the sockjs route handlers are currently exposed in a way that can be used externally like that easily, I'll play around with that.

It was never the intention of this library to support simultaneous use with a vanilla WebSocket.

Oh, I thought that was the only use case for the /websocket route since I didn't think the sockjs client ever used the raw websocket handler at all.