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

Alternative for implementing Sink on WebsocketConnection #15

Closed
Charles-Schleich opened this issue Mar 3, 2021 · 6 comments
Closed

Comments

@Charles-Schleich
Copy link

Hello !
So I'm looking for guidance regarding alternatives to implementing Sink for WebsocketConnection as raised in #3
I effectively need the behavior of being able to split up the stream into a tx and an rx so I can store the tx in a Arc<Mutex<Hashmap<ID, tx>>> and use in my application elsewhere.

stream.split() looked like a good way of achieving this but seeing as its not the plan for this crate to support Sink,
how can I achieve something like this ?

app.at("/ws")
    .with(WebSocket::new(|_request, mut stream| async move {
        let (tx,rx) = stream.split();
        // tx is then put inside my Arc-Mutex-HashMap and use it elsewhere 
        Ok(())
    }))
    .get(|_| async move { Ok("You're not a websocket, shoo") });

Or a better question, is there a better way of going about keeping track of multiple WebsocketConnection's such that I have access to the tx of them ?

@jbr
Copy link
Member

jbr commented Mar 3, 2021

I think patterns for this haven't entirely been exhausted, but the pattern I imagined was to share a broadcaster::BroadcastChannel or other stream of events with the handler and push events from the shared source stream into the websocket in the handler, instead of trying to hand the stream out of the handler. This makes disconnect and drop logic easier to reason about. In general, regardless of your event distribution approach, I was imagining the handler would subscribe to events, instead of using the stream outside of the handler.

I do think we might want abstractions on top of this crate that hide away the broadcast-and-filter logic, but continuing to use that general approach

@cryptoquick
Copy link
Contributor

So, wait, what are specifically supposed to do? I keep running into brick walls. Since I'm not able to split the Sink, I tried the WebSocketConnection inside a Mutex, but I don't think the Mutex ever unlocks if I'm awaiting within a while loop. I also tried using a RwLock, but because the Receiver is an Iterator, it also needs to be mutable. So, I'm kinda stuck.

@jbr
Copy link
Member

jbr commented Mar 11, 2021

Don't move the websocket connection out of the handler. Instead, send messages into the handler with an async mpmc channel of some sort and rebroadcast those to the websocket connection using send_json, send_string, or send. The handler is expected to be a long-lived async function that loops for the duration of the websocket

@cryptoquick
Copy link
Contributor

That's incredibly unnecessary, Rube-Goldbergian nonsense. We can just add the trait. It's not even a breaking change. Why does this have to be so difficult?

@jbr
Copy link
Member

jbr commented Mar 11, 2021

see #9 (comment), but one of the great things about this being a distinct crate from tide is that you can absolutely maintain a fork that has a different approach, and we will link to it from the tide readme

@cryptoquick
Copy link
Contributor

Alright, I've taken a walk and settled down a bit. I'm fine with maintaining the alternate library. Also, if you look at the code I was replacing in the link in #17, I did try with a separate thread, broadcast channel (like you mentioned), and while loop. But implementing the Sink trait instantly simplified the code and made it clearer to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants