Skip to content

Loading…

API to send to all/some websocket clients #193

Closed
davidw opened this Issue · 17 comments

4 participants

@davidw

The current websocket API does not provide a way for some other component of the system to send messages to client websockets, as far as I can tell.

I resolved the problem like so, but there is probably a better way:

https://github.com/davidw/ChicagoBoss/compare/expose_available_websockets

@evanmiller

Any thoughts, @mihawk?

@evanmiller

websockets_eval is a bad idea because it will block the gen_server doing lord-knows-what.

Seems like it'd be better to have some kind of websocket querying rather than return all of them unconditionally. Not sure what it would look like though.

@davidw

I'm not sure how to implement it, but one thing that might work well would simply be to make it so the WS controller implemented in the user's project has some way of receiving messages from the rest of the system. It's easy enough to save new sockets on joins, but there's no way that I can see for some other controller or server or whatever to send something to the WS controller to tell it to do something.

@davidw

Oops, you're right about blocking the gen_server - I moved the lists:map into the "api" function so that it runs in the calling process rather than the gen_server. For me it's "good enough", and I have a suspicion that something better would involve reworking things so that, as above, the individual web_socket handlers could receive messages from the rest of Erlang in an organized fashion.

@mihawk
@mihawk

what do you think if i add

boss_websocket:broadcast(ServiceUrl, {text, Msg}).
boss_websocket:broadcast(ServiceUrl, Tag,{text, Msg})?

boss_websocket:register(ServiceUrl, Tag, WebsocketID),
and boss_websocket:get(ServiceUrl, Tag),

you can tag a group of WebsocketId an retrieve them ?
the first is pub/sub pattern.
and all should work in clustered env.

it will be more convenient for newcomer to use websocket,

@mihawk

boss_websocket:register(Name, WebsocketId).
boss_websocket:broadcast(Name, Msg). with Msg::any(), Name::binary()
boss_websocket:get(Name).

is enough, and by default we register

boss_websocket:register(ServiceUrl, WebsocketId).

@davidw
@davidw

Any updates on this? I have working code in my branch, and I'd love to see something integrated sooner or later, as I'm beginning to collect a few too many branches for my tastes!

Thanks

@mihawk
@davidw

In some ways, I think it'd be nice if it were up to the developer to store incoming web sockets if that's what they want to do. That way we're not storing them in the case where people don't care. It does place a bit more of a burden on the developer to write the same code every time, which would be nice to not force people to do... Perhaps some kind of flag at init time? "Yes, we want to store all the websockets in order to be able to send to them".

@evanmiller

I think the websockets should be stored in Internal, but then we just need a way to access it for broadcasting. Maybe a new "handle_broadcast" function for the handlers, and a boss_service_worker:broadcast function that invokes it?

@davidw

I'm not 100% clear on what 'Internal' is referring to? Internal to CB is how it reads, but it's a bit ambiguous.

@evanmiller

"Internal" is the developer's application state variable. It can be whatever you want. You might do

init() ->
    {ok, sets:new()}

handle_join(_ServiceUrl, WebSocketId, _SessionId, WebSockets) ->
    {reply, Reply, sets:add(WebSocketId, WebSockets)}

Then the websockets are available to your handler module. Chan's API here is really nice because you can use the data structure that is most appropriate to your application. But it sounds like another function needs to be added so that you can explicitly send out messages, rather than having to wait for an incoming message.

@davidw

Ok, the state in the handler module - that's what I was referring to in my comment above. I think we're in agreement then - I'll try and rework things that way.

@zkessin

If this is merged I am going to close the issue

@zkessin zkessin closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.