Adding other chat backends?

[saethlin]

If I wanted to make a fork of tiny that can speak to other IRC-like chat services where would I start? I've poked around in the code and can't seem to find the part that reads from a connection and converts to an internal data structure.

[osa1]

If you want your backend to work with tiny's current backend (e.g. in a single
tiny process both IRC connections and your backend works) this'll require some
refactoring in tiny.

I was hoping to do something like this myself so maybe we can work it out.

To summarize current design:

• Conn module represents an IRC connection and it maintains the connection
  state: the actual TCP/TLS stream, current nick, connection status (to handle
  disconnects, handle peer introduction) etc. Each server tiny is connected to
  is a Conn instance.

  You drive Conn with read_ready() (called when the stream is ready for
  reading), write_ready() (called when the strema is ready for writing and
  there are bytes waiting to be sent), tick() (called on clock tick, i.e.
  every second), and some other functions for sending messages etc.

  Conn methods return ConnEv (stands for "connection event") which is
  defined in the same module. This is how you interact with IRC connections.

• The main module (lib.rs) maintains Conns, and depending on ConnEvs
  updates the TUI. It also maintains TUI (reads input events) and calls Conn
  methods for sending messages etc.

can't seem to find the part that reads from a connection and converts to an
internal data structure

This happens in Conn.rs, read_ready() method. This method returns ConnEvs
(fills the buffer passed by the caller), which are then handled by the caller
to update TUI. See the handle_conn_ev() method.

It seems to me like we need to refactor the code a little bit to support
different types of Conn.

Current Conn would become IrcConn. handle_conn_ev() would only handle
IrcConn events.

Another Conn type (e.g. XMPPConn) would also have the same set of types and
methods. E.g.

struct XMPPConn {}
enum XMPPConnEv {}
pub fn handle_xmpp_conn_ev(conn: &mut XMPPConn, ev: XMPPConnEv, tui: &mut TUI);

We could of course make Conn a trait, and event type an associated type.

Out of curiosity, what backend are you planning to implement?

In the meantime I think you can implement your client by using tiny's TUI as a
library. The TUI could actually be made a library, it doesn't have any
IRC-specific stuff, so it's somewhat reusable in other types if messaging apps.
(although some of the functions you may need may not currently exists in tiny's
TUI because it's implemented with tiny's use case in mind, but I think it's
easy to extend with new UI interactions)

[saethlin]

I'm interested in bringing all the various messaging apps I use into one terminal interface, where I can seamlessly switch between them. I currently use Discord, Slack, Facebook Messenger, SMS via Pushbullet, and IRC throughout the day and I think it would be cool to have them all inside Tiny. (obviously there will be some loss of functionality)

Following your suggestion I've been implementing a Slack backend. I think I got read_ready working, and so far these are my thoughts:

I think we should have a Conn trait that requires only a few methods. It probably needs at least one constructor but I don't know what that looks like (the two you have now are IRC-specific but the seem in the right vein). I think read_ready is a good way to get data from the internet into tiny, but I think some work needs to be done moving data in the other direction. Maybe we should have a write_ready that takes a buffer of TinyEv? I would like to keep the ConnEv system, and I think it's sensible to let different backends produce only a subset of them, and only handle a subset of TinyEv. Maybe they return something to indicate that event isn't valid?

[osa1]

I'll write more later but just wanted to point out

Maybe we should have a write_ready that takes a buffer of TinyEv?

We already have this, see Conn::write_ready (https://github.com/osa1/tiny/blob/master/src/conn.rs#L444-L455).

[saethlin]

Oh duh. I think I was just confused because it doesn't look like that method processes the events.

[osa1]

I'm interested in bringing all the various messaging apps I use into one
terminal interface, where I can seamlessly switch between them. I currently
use Discord, Slack, Facebook Messenger, SMS via Pushbullet, and IRC
throughout the day and I think it would be cool to have them all inside Tiny.
(obviously there will be some loss of functionality)

This is great! I was thinking of doing the same for slack (which we use at
work) and gitter (some OSS I use use it).

OK so I have this idea:

Instead of connections returning (or filling in) a vector of events, for TUI
updates I think we could let connections update their TUI directly. This removes
the need for connection-specific events.

Connections have three update functions:

• tick() called every second
• read_ready() called when connection should do reading
• write_ready() called when connection should do writing

These handle TUI updates themselves.

For updating tiny's state (rather than a connection's state) we implement
TinyEv. TinyEv is what a connection returns to tiny's event loop.

Then for passing input from TUI to connections, we implement TUIEvent.

In summary it'd look like this:

use logger::Logger;
use tui::TUI;
use tui::tabbed::MsgSource;

/// Every `Conn` needs to be `Evented` to be useful
trait Conn {
    /// Called when `Conn`s socket is ready for reading.
    ///
    /// * `tui`: TUI handle that can be used to update tabs associated to this connection.
    /// * `evs`: Events for tiny to handle.
    /// * `logger`: Logger handle for this connection.
    ///
    fn read_ready(&mut self, tui: &mut TUIHandle, evs: &mut Vec<TinyEv>, logger: &mut Logger);

    /// Called when `Conn`s socket is ready for writing and the socket is registered for write
    /// events.
    ///
    /// Arguments have same meaning with `read_ready`'s arguments.
    ///
    fn write_ready(&mut self, tui: &mut TUIHandle, evs: &mut Vec<TinyEv>, logger: &mut Logger);

    /// Called every second.
    fn tick(&mut self, evs: &mut Vec<TinyEv>, logger: &mut Logger);

    /// Called when a command was sent to this connection.
    fn handle_cmd(&mut self, cmd: String, args: Vec<String>, evs: &mut Vec<TinyEv>, logger: &mut Logger);

    /// Called when an input was received in a tab that's associated with this connection.
    /// Implementations can return events to tiny using `evs` parameter.
    fn handle_tui_event(&mut self, ev: TUIEv, evs: &mut Vec<TinyEv>, logger: &mut Logger);
}

pub enum TinyEv {
    /// Drop this connection
    ConnectionClosed,
    // what else?
}

pub enum TUIEv {
    Input {
        msg: Vec<char>,
        from: MsgSource,
    }
}

/// A handle that allows updating tabs for a `Conn`.
pub struct TUIHandle {}

We then implement Conn for each backend, and extend TinyEv and TUIEv for
as we need more functionality for backends.

Does that make sense? How about this plan: I create create a new branch and
implement this trait and port current IRC backend to it. You can then try to
implement another backend using this API and we can see what's working OK and
what's not, update, and repeat.

[saethlin]

I like that you've shrunk the requirements for a connection (the existing Conn has a lot of public methods), but I don't understand the role of each of these methods. Here's how I see them:

read_ready will be called when tiny thinks there are messages to be read from me. I store them internally, then modify the TUI directly when I can and pass tiny an event when I need something more complicated.

write_ready Where's my input here? In read_ready these same arguments were outputs. Do I read from the vector of TinyEv, then add the sent message to the TUI?

tick lets a Conn do timeout handling. This may be stubbed out by non-IRC backends because they run on websockets that do their own timeout stuff.

handle_cmd this is how do my /join or whatever is appropriate for this kind of connection

handle_tui_event I'm not sure what I can do with this, don't I already know about all inputs received in tabs associated with me because that's my job?

The plan sounds good to me. I've been working on a Slack backend, and so far it looks not all that hard (I've written Slack stuff before). I think I've almost got something together for PushBullet, but their docs seem very incomplete so that could get interesting.

I can already tell you that the batched setup that you have for dealing with IRC connections does not play very well with the websocket interface used by nearly every other chat service. Event loops and handlers are the easiest thing to do, so right now I have a handler that just sits in its own thread and pushes into an Arc<Mutex<VecDeque<slack::Event>>> as events arrive. It might be worthwhile to mutate the TUI as events appear. Maybe the constructor of a Conn is passed an Arc<Mutex<TUIHandle>>?

[saethlin]

Feedback on the proposed Conn trait:

Some commands may produce an error. handle_cmd should either get a handle for the TUI or we should support error notifications via a TinyEv.

EDIT: Just noticed some commands want to know which channel we're in. More reason to have a handle to the TUI I think.

[osa1]

As far as I understand you're thinking a futures/threads based API whereas I
was thinking of a mio-based one.

In mio-style code you can't tell if a socket your backend is using have data to
read or have space in its buffer to write. So that's why we have read_ready
and write_ready. Those are called by the mio event loop and they notifies
that the socket you're using is ready for reading or writing.

Instead if we design threads-based API then every backend can take care of
their sockets. In a backend both futures and mio could be used.

If we design a futures-based API ... well first of all futures API is terrible
IMHO. Secondly, because futures supposed to work with event loops but also with
thread pools you have to design your API with threading in mind. So for example
you need to pass stuff wrapped with Arc<Mutex<...>> rather than just passing
&mut .... This is possible in mio-based design because clearly event loop
calls read_ready, write_ready and other methods in a loop, there's no
concurrency at all.

read_ready will be called when tiny thinks there are messages to be read from
me. I store them internally, then modify the TUI directly when I can and pass
tiny an event when I need something more complicated.

This is correct. tiny thinks there are messages to be read from you when mio
thinks so as described above.

write_ready Where's my input here? In read_ready these same arguments
were outputs. Do I read from the vector of TinyEv, then add the sent message
to the TUI?

You get your inputs from TUI in handle_tui_event, and fill in your outgoing
message buffers according to TUI inputs. However, this doesn't mean you can
write the data directly to the socket (the socket may not have any space left),
that's done in write_ready.

So

• inputs are provided by TUI in handle_tui_event
• actually sending bytes is done in write_ready which is only called when
  your socket is ready for sending data (which is decided by mio)

tick lets a Conn do timeout handling. This may be stubbed out by non-IRC
backends because they run on websockets that do their own timeout stuff.

Hmm so they use threads and/or futures. OK.

handle_cmd this is how do my /join or whatever is appropriate for this kind
of connection

Exactly.

handle_tui_event I'm not sure what I can do with this, don't I already know
about all inputs received in tabs associated with me because that's my job?

This is where you get inputs from TUI. So suppose someone entered hello world
in tab #foo. You get that information as a TUIEvent in this method.
Depending on the backend this may mean sending a message to channel #foo, for
example.

I can already tell you that the batched setup that you have for dealing with
IRC connections does not play very well with the websocket interface used by
nearly every other chat service

Right.. So maybe designing an API around a mio-based event loop is not a good
idea (although I suspect it's possible to implement adapters from futures-based
APIs to mio-based APIs and the other way around although I hvaen't tried this
yet).

As mentioned above, nice thing about about the current design is you don't have
to think about concurrency, you can share things as &mut ... instead of
Arc<Mutex<...>>. But maybe this is something we can let go to have a more
convenient API.

The options I see:

• Assuming libraries you need to use use futures: study futures; try to port
  the current IRC backend to it etc. and come up with a futures-based design.
  In this API we'd probably just pass a tokio Handle to the backends on
  initialization and the rest would be done entirely in the backends.

• We design a threads-based API. In this API each backend is responsible for
  doing I/O multiplexing etc. so they can decide whether to use mio or
  tokio or more threads internally.

  I kinda dislike this option because with this scheme I'd have to spawn a
  thread for each IRC server. Currently tiny runs in one thread no matter how
  many servers you connect to which I like.

• Something else?

Some commands may produce an error. handle_cmd should either get a handle for
the TUI or we should support error notifications via a TinyEv.

EDIT: Just noticed some commands want to know which channel we're in. More
reason to have a handle to the TUI I think.

Ah, this makes sense. In fact current handle_cmd already has a handle to the
TUI.

[osa1]

Btw is your code open source yet? I'd love to take a peek to get a better understanding at what we need from the backend API.

[saethlin]

I'm totally down for doing this with mio. I'm just not sure how mio figures out that my connection can be read from or written to...

Code is here. I butchered my conn.rs quite a bit, so the code I've been working with doesn't get to the borrow checker when I cargo check it. Should probably fix that...

[osa1]

I'm totally down for doing this with mio. I'm just not sure how mio figures out that my connection can be read from or written to...

mio just decides whether the socket your connection is using can be read of
written to.

This is how event loops work, under the hood it uses epoll() system call (or
poll(), select() etc.).

When you want to send data to the remote side of your connection, you tell the
event loop to notify you when your socket is readay for writing, by calling
your write_ready.

Let's see how tiny does this. Suppose we want to send a PRIVMSG. We call this
Conn method:

pub fn privmsg(&mut self, target: &str, msg: &str) {
    self.status.get_stream_mut().map(|stream| {
        wire::privmsg(stream, target, msg).unwrap();
    });
}

Which uses stream::Stream's Write impl.:

impl<'poll> Write for Stream<'poll> {
    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
        match *self {
            Stream::Tcp(ref mut s) =>
                s.write(buf),
            Stream::Tls(ref mut s) =>
                s.write(buf),
        }
    }
}

Note that Stream has a reference to mio event loop to be able to register
itself for write events (so that the event loop notifies it when its socket is
ready for writing via the write_ready method).

Now a stream can be either TlsStream<TcpStream> or TcpStream. Let's skip a
layer of indirection and look at TcpStreams Write impl which is used in
this method:

impl<'poll> Write for TcpStream<'poll> {
    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
        // TODO inefficient when the socket is already ready for writing
        self.out_buf.extend(buf);
        reregister_for_rw(&self.poll, self.inner.as_raw_fd());
        Ok(buf.len())
    }
}

See how TcpStream also has a reference to the event loop. In this code we
don't actually send any data, we just register our socket for write events and
then write the data to our outgoing message buffer.

Then when the socket is ready for sending, the event loop calls
TcpStream::write_ready() (actually it calls Conn::write_ready(), which in
turn calls TcpStream::write_ready):

impl<'poll> TcpStream<'poll> {
    pub fn write_ready(&mut self) -> io::Result<()> {
        let to_send = self.out_buf.len();
        match self.inner.write(&self.out_buf) {
            Ok(bytes_sent) => {
                self.out_buf.drain(0..bytes_sent);
                let register = if bytes_sent == to_send {
                    reregister_for_r
                } else {
                    reregister_for_rw
                };
                register(&self.poll, self.inner.as_raw_fd());
                Ok(())
            }
            Err(err) => {
                reregister_for_rw(&self.poll, self.inner.as_raw_fd());
                Err(err)
            }
        }
    }
}

This is where we do the actual sending and update our outgoing message buffer.

Hopefully this clarifies mio-based API.

I'm not saying this good or convenient or anything like that. I'm just
explaining the design. I suspect that this is not as convenient for other
libraries (especially fuutres-based ones) so we may want to design something
different.

[osa1]

Hmm I think this is not going to work -- for this API to work you'd need read_ready and write_ready methods for the entire stack. So, if you're using slack, which probably uses reqwest under the hood, you need write_ready method for slack which will need write_ready for reqwest.

I think we should go with threads-based API and let the backends choose whatever method they like. Just fork a thread for each backend, pass a TuiHandle (which lets backends update their tabs no other tabs, so this is not shared and can be directly moved to a thread), and we're done.

Only problem is we need to figure how to poll TuiHandle for TUI events.

[saethlin]

I now own a chromebook so I really want these other backends in place.

I'm tempted to say that we should figure out an interface between connections and the rest of tiny that's decently clean and just run with it for now. If we have threads all over, I'm sure they can be replaced with the upcoming async features (we're on nightly already).

The biggest hurdle for me on getting something up and running is that all of tiny's internals are tightly locked to the way a Conn works. I'm not sure I'm up to the task of moving the current Conn into the new implementation. Is this something you're interested in doing? If not I could really do with some guidance.

It just occurred to me that I might be able to hack the new trait-based interface into tiny by adding a new member to the Tiny struct.

[osa1]

I've been working on this and I started to think that we should implement a futures-based API and implement backends on top of that.

I think for most backends we need one of these three things:

• UDP/TCP/TLS connections. irc crate does TCP and TLS part using futures so it looks possible.

• Websocket connections. websocket crate does this using futures: http://cyderize.github.io/rust-websocket/doc/websocket/client/builder/struct.ClientBuilder.html#method.async_connect

• HTTP(S) connections. reqwest does this using futures when compiled with a feature flag: https://github.com/seanmonstar/reqwest/blob/master/src/async_impl/client.rs#L117

So it seems to me that if we use futures we don't have to spawn threads per backend.

In this design we'd pass a Receiver<TuiEvent> and a tokio Handle to backends. Using Handle they can spawn async tasks, using the receiver they can read TUI events.

All backend methods would also get a Arc<Mutex<TuiHandle>> that allows them to modify their tabs or create new tabs.

I'll try to design the API in more details and try to port current IRC backend but there's one problem that I don't know how to solve; I don't know how to read stdin using tokio and futures.

Once I figure that out I should be able to try this design.

The biggest hurdle for me on getting something up and running is that all of tiny's internals are tightly locked to the way a Conn works. I'm not sure I'm up to the task of moving the current Conn into the new implementation. Is this something you're interested in doing?

Yes, and I can also provide guidance ... However I'm a bit busy these days so I can't give any ETAs..

[osa1]

So I've been studying futures lately and I think I'm getting back to the thread-based API idea. futures ecosystem (tokio and co) are such a mess and it's impossible to do even the simplest things like reading stdin as &[u8]. (I think it should be possible, but I couldn't do it after hours of research)

So maybe something like this would work:

• Provide a TuiHandle to backends that allows modifying the backend's tabs.
• Provide a Receiver<TuiEvent> to backends that allows reading TUI events.

In addition, I want to keep current backend working without spawning any threads so another interface should be supported.. More on this later.

[saethlin]

I've been fiddling with futures and they look like a hot mess to me too. My compile errors see unintelligible and I can't find anyone on IRC or discord that can help. Fortunately, this now exists: https://crates.io/crates/may

[osa1]

I was excited about MAY too, but unfortunately it seems like it's deeply broken: Xudong-Huang/may#6 the problem is not specific to MAY, we just can't implement work-stealing threads in Rust because of issues with TLS.