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

Server Implementation Checklist #124

Open
14 tasks
ctrlaltf24 opened this issue Oct 1, 2021 · 8 comments
Open
14 tasks

Server Implementation Checklist #124

ctrlaltf24 opened this issue Oct 1, 2021 · 8 comments

Comments

@ctrlaltf24
Copy link
Collaborator

ctrlaltf24 commented Oct 1, 2021

Development on hold, someone else is welcome to take over

It's that time @1c3t3a , might want to make a branch. How servers are implemented informs discussion about what the best interface method is (for 0.3.X).

TBD:

  • Polling server added
  • Websocket server added
  • Socket implemented
  • Introduce tokio v1 internally
    • DO NOT expose async functions
    • Interface should still be synchronous externally.
  • Tests written
    • Unit tests
    • Integration tests against rust_socketio client
    • Integration tests for client against server
    • Integration tests against nodejs reference impl
    • Performance tests
    • Maintain coverage
  • Interface written
@ctrlaltf24
Copy link
Collaborator Author

ctrlaltf24 commented Oct 4, 2021

@1c3t3a It would be easier to implement 0.4.0 if we used async internally, but didn't expose. That alright with you? Async internally but synchronous end-user interfaces? Using https://docs.rs/futures/0.3.17/futures/sink/index.html https://docs.rs/futures/0.3.17/futures/stream/index.html# internally, then consuming the sinks into callbacks.

@ctrlaltf24
Copy link
Collaborator Author

ctrlaltf24 commented Oct 7, 2021

We might be able to leverage https://tokio.rs/tokio/topics/bridging to use async internally but expose a sync interface. Not sure how that would work for people with conflicting tokio versions though.

Might be smarter to expose this as a feature

@1c3t3a can you please make a 0.4.0 branch?

@1c3t3a
Copy link
Owner

1c3t3a commented Nov 12, 2021

Yes, just opened it!

@ctrlaltf24
Copy link
Collaborator Author

ctrlaltf24 commented Nov 17, 2021

@1c3t3a how does this look for a the closure based callback interface for the server? This interface will involve several Arc Mutex locks, after/during implementation we can discuss other interfaces that are more rusty. Planning to make the server async (behind a feature).

async fn main() {
    let server = ServerBuilder::new(4202)
        .on("connection", |_, client, server| {
            client.on("message", |message, client, server| {
                client.emit_with_ack("hello awk", |_, client, server| {
                        // awked
                }).await?;
            });
            client.on("heartbeat", |_, client, server| {
    
            });
            client.on("error", |message, client, server| {

            });
            client.on("close", |_, client, server| {

            });
            client.emit("hello client").await?;
        })
        .connect().await?;
}

or

async fn main() {
    let server = ServerBuilder::new(4202)
        .on_connection(|_, client, server| {
            client.on_message(|message, client, server| {
                client.emit_with_ack("hello awk", |client, server| {
                    // awked
                }).await?;
            });
            client.on_heartbeat(|client, server| {

            });
            client.on_error(|message, client, server| {

            });
            client.on_close(|client, server| {

            });
            client.emit("hello client").await?;
        })
        .connect().await?;
}

While the latter is cleaner, the first is more consistent with both the js implementation and the client

@1c3t3a
Copy link
Owner

1c3t3a commented Nov 22, 2021

I would honestly prefer the former, as it sticks more to the socketio API. Also the latter could be more difficult when reacting to a custom event. And I honestly like the Into<Event> Generic bounds which allow passing in strings in a type safe manner. What do you think @nshaaban-cPacket?

@ctrlaltf24
Copy link
Collaborator Author

Agreed, the former make more sense. I'm writing the code in such a way that should enable us to write different API interfaces in the future (like more rusty/streams ones) .

@1c3t3a
Copy link
Owner

1c3t3a commented Oct 20, 2022

With 0.4.0 being about reconnect refactor, we should either rename this to 0.5.0 or close.

@ctrlaltf24 ctrlaltf24 changed the title 0.4.0 Checklist Server Implementation Checklist Oct 20, 2022
@ctrlaltf24
Copy link
Collaborator Author

Looks like someone beat us to it https://github.com/Totodore/socketioxide implemented as a tower layer, interesting

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

2 participants