sdroege/async-tungstenite

Question about chat server example

mlodato517 opened this issue · 5 comments

mlodato517 commented

This is a question and not an issue so I'm sorry for putting it here. If there's somewhere else to put it, I'll gladly close this and move it there!

I copied the chat server example to set up a multiplayer game where all player clients report their current position and the server broadcasts that to its peers so they can draw each other. One thing I noticed was, when one client closed, all clients were disconnected. I believe this is because the websocket close message that is sent by one client is then broadcast by the server to the other clients. In my application, I updated the code to just not broadcast the message if it was a close message.

I'm wondering two things:

  1. Is my workaround just a hack? Is it best practice to broadcast the close message so other clients know one client has closed? Or should the server consume the close message from one client and broadcast a non-close message to the other clients alerting them of the close? In my experience, the other clients just got the close message and went, "Oh, the server closed. Bye!".
  2. Should the example be updated? I'd be happy to do this so I could get PR feedback on the correct way to do it. I'm currently working my way through the tokio docs but I honestly have a very hard time understanding how this all fits together so far :-)
sdroege commented 
1. Is my workaround just a hack? Is it best practice to broadcast the close message so other clients know one client has closed?

I think that's the correct way of doing it, yes. Broadcasting the normal close message will close the connection for everybody, which is usually not what you want.

2\. Should the example be updated?

I think that was be useful, thanks :)

  • 👍1
mlodato517 commented

Hmm, this is actually interesting though. If I run your server-client example, I don't see this behavior (where closing one client closes the rest of the clients). I'm not sure if this is specific to when the client is a JS websocket client or something? I'm interested in your intuition on this:

  1. I started the server:
cargo run --features="async-std-runtime" --example server 127.0.0.1:12345
  1. I started a rust client:
cargo run --features="async-std-runtime" --example client ws://127.0.0.1:12345/
  1. I added an index.html file to examples:
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
  </head>
  <body>
    <script>
      document.addEventListener('DOMContentLoaded', function() {
        const ws = new WebSocket('ws://127.0.0.1:12345');

        ws.addEventListener('open', function() {
          ws.addEventListener('message', function({ data }) {
            console.log(data)
          })
          ws.addEventListener('close', function() {
            console.log('closed')
          })
        })
      })
    </script>
  </body>
</html>
  1. I opened index.html in two windows - one in FF, one in chrome just be safe.
  2. I typed into the rust client and saw a blob logged in the console of both windows to ensure the broadcast was working.
  3. Now, if I close either browser tab, it closed the other browser tab and the rust command-line client.

But if I connect with two rust clients and close one, the other client doesn't close. Also if I connect with one rust client and one JS client and then close the rust client, the JS client isn't closed. Do you think this is a JS bug or something instead of an issue with broadcasting the close message?

Tangentially I noticed that if you try running the command listed at the top of the example, you get an error:

error: target `client` in package `async-tungstenite` requires the features: `async-std-runtime`
Consider enabling them by passing, e.g., `--features="async-std-runtime"`

Do you want me to update the commands in the example or should we leave it as to keep the commands shorter because the error is so well written?

sdroege commented

Hmm, this is actually interesting though. If I run your server-client example, I don't see this behavior (where closing one client closes the rest of the clients). I'm not sure if this is specific to when the client is a JS websocket client or something? I'm interested in your intuition on this:

Maybe no Close message is sent in that case but the underlying TCP connection is just closed? If you just quit the Rust application it will simply stop and not have the opportunity to cleanly close the websocket connection (i.e. sending the Close message). And because of that, this message also wouldn't be broadcasted. Sending the Close message is optional and you'd have to do that explicitly in the Rust code.

Are you saying your code is not behaving the same as the examples, can you share your code in that case? :)

Do you want me to update the commands in the example or should we leave it as to keep the commands shorter because the error is so well written?

Updating those would be a good idea, thanks :)

mlodato517 commented

If you just quit the Rust application it will simply stop and not have the opportunity to cleanly close the websocket connection

Ah! That could very well be it. Great, I'll go ahead with those updates then.

sdroege commented

I guess this can be closed now? :) If not tell me and I'll reopen.

  • 🎉1