API Discussion: becoming lightweight and feature-full (through hyper, mio, net2 and more)

[illegalprime]

This seemed like a good place for this. I wanted to flesh out a few things I was thinking about.
There are many websocket libraries out there today but this one is the coolest, and it can be cooler!
There are many missing features and pain points of the library that I wanted to discuss and I thought I would list them here. We should also include some features from other libraries and become the best ws library out there!

Features:

• HTTP Server integration
  • Take hyper requests and turn them into websockets
  • integration with hyper and others can be kept under a feature.
  • easier routing
• custom headers
  • Cookies is probably useful, etc.
• non-blocking
  • simple, use the net2 crate
• mio support
  • easy, but API should be decided first
• protocols
  • easy, needs API change
• bare bones
  • easier user control of the dataframe sent over the wire
  • a lot of this is just more documentation
• thread pool
  • probably behind a feature toggle, not a priority, seems useful
• autobahn tests
  • add permessage-deflate

Pain Points:

• maybe using hyper is overkill. still not 100% sure.
  • http-muncher or httparse + some header library might be OK
  • we really don't need anything more than stdlib, see websocket-stream
  • we might be able to get away with maybe 1 dependency
• not so precise error handling scheme, although this is a low priority
• more documentation
• more fluent API
  • this is mostly true but I think there are some places that could be more readable
• less unecessary copying (there is still some going on!)

Future:

• I'm a worried about supporting future revisions to the standard, the code now isn't so flexible.

This mostly spawned out of me trying to fix #53, and I thought we should discuss some API before I sink more time into it. If we're all clear on what we want it to look like we can easily split up work and people can more easily contribute.

So @cyderize to start off do you think this list is complete? I have some API ideas I'll post soon.

[cyderize]

@illegalprime, it's awesome that you're contributing - thanks so much!

The only other thing I can think of is specifically supporting permessage-deflate, and also making the unit tests more comprehensive.

With regards to custom headers though, I'm not sure what you're referring to there. I mean, you can already access Request.headers and Response.headers to manipulate whatever hyper headers you need, so adding headers is already possible. Or are you talking about something else?

My concern with not using hyper is that we'd probably end up having to use string-type or binary headers, which I'm not really a fan of. I quite like hyper's headers, and I wish that they were in their own crate.

I'm looking forward to seeing your API ideas.

[illegalprime]

Not a problem! Just want to get a foot in in the open source community.
I'm also fine with hyper, their headers are pretty cool.

Clients

Currently

let url = Url::parse("ws://127.0.0.1:1234").unwrap(); // Get the URL
let request = Client::connect(url).unwrap();          // Connect to the server
let response = request.send().unwrap();               // Send the request
response.validate().unwrap();                         // Ensure the response is valid
let mut client = response.begin();                    // Get a Client

This might not be great for a few reasons:

• There is no nice way to configure the websocket without going through headers (like extensions).
• There is no nice way to make a websocket out of something other than a WebSocketStream
  • There is, but it's with ::new(), which is doesn't even verify the read/write sockets are talking websocket (not that there's a way to).
• This requires an intimate knowledge of the websocket standard, which:
  • could be changed in the future, causing an API break
  • more importantly users have to know about the standard, a barrier to entry
  • this is more work than any other library (probably) makes you do
• a client can be created without validating the response
  • this can forced with the type system
  • can be easily forgotten
  • requires somewhat intimate knowledge of the spec
• it's so many lines!

Proposal

Overview of Options

A list of possible websocket configuration options

• protocols: a list of strings, the protocols
• extensions: a list of strings, the extensions
• ssl_context: a custom (not the default) ssl context
• stream: a custom stream to use (not the default)
  • this can be either (R: io::Read, W: io::Write) or S: io::Read + io::Write

Semantics

::connect() here means start the handshake immediately and only return when an error has occurred or a websocket connection was established

::new() means create an intermediate struct holding all the information needed for a connection but do not write to the stream yet.

This is also more consistent with the standard websocket client implementation (except in that one new automatically attempts to connect).

Possible API No. 1: Option Strutcs

// `connect` here means immediately start the handshaking process
let mut client = Client::connect("wss://echo.example.org", &Opts {
    // An optional ssl context, other than the default
    ssl_context: None,
    // a list of protocols
    protocols: ["image/jpeg", "echo"],
    // a list of extensions
    extensions: ["gzip"],
    // the stream to talk over other than the default.
    // either an (Read, Write) or a Read + Write
    stream: None,
    .. Default::default()
})
.unwrap();

// `new` means do not start the handshake and return an intermediate client
let client_template = Client::new("wss://echo.example.org", Default::default());
// .. do some work
let mut client = client_template.begin().unwrap();   // .begin takes a borrow of self

Possible API No. 2: Builder Syntax

// Gradually "build" a websocket and connect
let mut client = Client::build("wss://echo.example.org")
    .protocols(["echo"])
    .extensions(["deflate"])
    // you can get what `new` does for free by not calling this:
    .connect()
    .unwrap();

// Maybe a Client::connect() and Client::new() for convenience

Servers

Currently

The server implementation is good, but it doesn't allow you to integrate with other stuff like #53 mentions. Can you tell I'm getting tired of writing by now?

Proposal

Basically many things will implement .into_ws() which is a function that returns a representation of the handshake request if one exists. Then one can call accept and reject to turn the stream into a client.

With Hyper

Server::http("0.0.0.0:0").unwrap().handle(move |request, response| {
    match request.into_ws() {
        Ok(upgrade) => {
            if upgrade.protocols().contains("echo") {
                let mut client = upgrade.accept().unwrap();
                // some ws stuff
            } else {
                upgrade.reject();
            }
        },
        Err((request, _)) => {
            // some http stuff
        },
    }
});

Implementation

The trait:

pub trait IntoWs {
    fn into_ws(self) -> Result<WsUpgrade, (Self, Error)>;
}

The WsUpgrade struct:

pub struct WsUpgrade {
    // Note: consuming self
    fn accept(self) -> Result<Client> {}
    fn reject(self) -> Result<()> {}
    // Some convenience methods
    fn protocols() -> &[&str] {}
    fn extensions() -> &[&str] {}
    fn origin() -> &str
    /* probably more stuff */
}

Default Server

Now the server that comes with the package will just automatically call .into_ws(). So it will be more like:

let server = Server::bind("127.0.0.1:1234").unwrap();
for connection in server.connections() {
    if upgrade.protocols().contains("echo") {
        let mut client = upgrade.accept().unwrap();
        // some ws stuff
    } else {
        upgrade.reject();
    }
}

We can just let people implement IntoWs for hyper and other libraries, putting these implementations behind a feature toggle (although not for hyper since it's already a dependency).

Miscellaneous

• server/receiver and client/receiver can be one module.
• server/sender and client/sender can be one module.
• server/request and client/request can be one module.
• server/response and client/response can be one module.
• implement permessage-deflate

Wrapping Up

I think this API will last a while and is reasonable, what do you think about it?

[illegalprime]

And another thing, I'm thinking Client<R, W, D> should be changed to Client<S, D> like so:

pub struct Client<S, D> {
    stream: S,
    _dataframe: PhantomData<fn(D)>,
}

pub trait Stream {
    type R: Read;
    type W: Write;

    fn reader(&mut self) -> &mut Self::R;

    fn writer(&mut self) -> &mut Self::W;

    fn split(self) -> (Self::R, Self::W);
}

impl<S> Stream for S
where S: Read + Write {
    /* ... */
}

impl<R, W> Stream for (R, W)
where R: Read,
      W: Write {
    /* ... */
}

The implementation in the crate will actually create a Client<(WebSocketStream, WebSocketStream), DataFrame> because it is easier to split a (R, W) than a R + W. This makes it much easier for users to use different streams in the client. A similar thing would be needed for the server implementation.

[cyderize]

@illegalprime I really like your proposal, I think it would really improve the library a lot!

For the client API, I'm leaning towards API No. 2 (builder pattern) because hyper uses builder syntax, and it's probably not a bad idea to follow that. I might have a go at implementing each and see which I prefer, since I have some time to work on this now.

The WsUpgrade trait looks good, that should allow for much tighter integration with other libraries.

Thanks for all your hard work!

[jaredly]

WsUpgrade would be super nice to have! Looking forward to it

[avitex]

Any new²+s on this? Happy to help out, as I need rust experience.

[illegalprime]

Hey @avitex I just finished with my exams, so I'll be able to put more attention towards this.
Give me a bit to reorient myself, but I'd love some help if you are serious?

[avitex]

@illegalprime Sure am. Keep in mind I am relatively new to the rust eco-system, however that's the point of working with someone who is experienced.

[illegalprime]

@avitex I think a straightforward task on this ticket is to begin work on the client builder.
This simply means being able to make a new client based on the builder syntax described above:
being able to configure the client, save that configuration, and then connect to the ws server without
having to do this other weird 'return a request and then ask the user to send it' stuff all falls under that.
If you decide to go this route I think the convention is to append Builder to the structs that do the
building, so in this case it will be something like ClientBuilder.

Another thing that I think should be straightforward but maybe not it to merge

1. client/request.rs and server/request.rs
2. client/response.rs and server/response.rs
   They really should not be different things.

Let me know what you'd be interested in and of course ask if you have any questions. Reach me by email (listed on my profile) or mention me somewhere on GitHub, you can also start a PR against my fork and we can discuss your progress there :)

[illegalprime]

It's done.