aatxe / irc

the irc crate – usable, async IRC for Rust
Mozilla Public License 2.0
525 stars 100 forks source link

Sends seem to be buffered #246

Open bemug opened 1 year ago

bemug commented 1 year ago

Might be a noob question, I'm learning Rust.

When using tokio::time::sleep to delay each send_privmsg, it sends them all at once instead of sending one, then delaying, then sending one, etc. Here is a reporducing example, where you can see all message got sent at once after waiting for 5 seconds.

use futures::prelude::*;
use irc::client::prelude::*;
use tokio::time::{sleep, Duration};

#[tokio::main]
async fn main() -> irc::error::Result<()> {
    let config = Config {
        [...]
    };

    let mut client = Client::from_config(config).await?;
    client.identify()?;

    let mut stream = client.stream()?;
    let sender = client.sender();

    while let Some(message) = stream.next().await.transpose()? {
      match message.command {
        Command::PRIVMSG(ref target, ref msg) => {
          for i in 0..5 {
            sender.send_privmsg(target, i)?;
            sleep(Duration::from_secs(1)).await;
          }
        }
        _ => (),
      }
    }

    Ok(())
}
Sildulir commented 1 year ago

Might be a noob question, I'm learning Rust.

This behavior is ~somewhat~ surprising unless you understand exactly what this crate is doing so no it isn't a noob question.

In short this crate make it so that the send_<message_type>() and send() methods are simply sending the message to an Outgoing future that might send them the next time it is poll-ed. By default, when you make a ClientStream, the Outgoing is polled by the poll_next() method of the ClientStream (so the next time your application is looking for an incoming message).

Therefore the only time messages can be sent is at the start of the next loop iteration (it will then send as many messages as possible without any waiting in between). Hence the behavior you see.

Therefore the solution would be to separate the Outgoing prior to making the stream (with the ougoing() method of Client) and driving it in a separate thread (see tokio::spawn().

I personally believe that sends should just send the message through the network connection instead of using such a scheme but there probably is a reason not to do that here... (might be historical though)

8573 commented 1 year ago

I personally believe that sends should just send the message through the network connection

I wonder whether the synchronous API in version 0.13 would work for this. (I imagine one would be giving up something by using 0.13 rather than 0.15, but I don't remember what.)

aatxe commented 1 year ago

The design is definitely historical, and I'd welcome PRs looking to fix it. It might be something I look at myself if I can find the time, as well.