Client Protocol Upgrades

[seanmonstar]

(For handling server protocol upgrades, see #1323)

HTTP/1 allows starting a different protocol with an HTTP/1 request, and the desired protocol in an Upgrade header. We should support this.

Edit: removed bad API proposal.

[letmutx]

This looks great!

The channel based approach we discussed also seems to be equally ergonomic and a looks much simpler to implement without adding another type parameter to virtually all types in proto. Is there any reason you prefer this over that?

[seanmonstar]

Well, shoot. The reason I liked this was for the end user API, but I think it might be impossible to do in 0.11 because:

• Body is Send + Sync
• We can't enforce the transport is Send + Sync, so we can't add Box<AsyncRead + AsyncWrite> to the internals of Body.

What exact user API were you thinking with channels?

[letmutx]

struct Upgrade<T: AsyncRead + AsyncWrite> {
    io: T,
    response: MessageHead<RawStatus>
    handle: Handle
}

fn main() {
    let client = Client::configure().enable_upgrades().build();
    let upgrade_rx = client.upgrades();
    let upgrade_task = upgrade_rx.then(|upgrade: Upgrade| {
        do_something(upgrade);
        Ok(())
    });
    let handle = client.handle();
    handle.spawn(upgrade_task);
    let req = Request::new(Method::Get, "https://example.domain/".parse()?)
    .with_header(Connection::upgrade())
    .with_header(Upgrade::websocket());
    let req = client.request(req).and_then(|res| {
        handle_req(res);
        Ok(())
    });
    core.run(req).unwrap();
}

But come to think of it, that is a lot of boilerplate for a single request. My use case is kind of like a proxy I would make a stream of requests, so I am biased.

[seanmonstar]

If there is an upgrade, what does res look like? Does a Response get generated and called? What goes in the Upgrade struct?

What I preferred about the initial proposed API is that handling an upgrade was local to the Response, and so obvious. My concern with configuring the Client is that it can happen in different places, and thus it might not be obvious that upgrades are handled...

[letmutx]

Does a Response get generated and called?

Yes.

What goes in the Upgrade struct?

Upgrade struct is right at the top of the example.

[seanmonstar]

Upgrade struct is right at the top of the example.

Ah OK, it wasn't in the original message I got in email, didn't notice the edit.

---

So, a user would receive the Response like normal, and would be able to inspect it, and if it wasn't an upgrade, try to read the body. Regardless, if it is an upgrade, the upgrades receiver would also receive an Upgrade...

I suppose it allows working without a breaking change, but my personal opinion is that it kind of feels far away from the rest of the code. Hm, does that even matter? I suppose that the underlying connection is being used in an upgrade perhaps doesn't matter to the other place that received a Response...

[seanmonstar]

Ok, sorry it's taken so long to get back here, I haven't had much time to focus on this feature, but it's now needed in Conduit, so it's got my attention again.

I've started with the idea that the client could use a lower-level API of running HTTP over a single connection, and there is where upgrades can be handled at first (as well as custom pools and whatever else). I feel that designing an API to work directly with the higher level Client, which manages connections for you, is holding back this feature work, so that's why I've gone this direction. I think it could be possible to eventually design an API that works with the higher level Client, but here's a proposal to get things started: #1449 (comment)

[letmutx]

I too tried to take a stab at it a while ago. The problem seems to be with the pool. At the dispatcher level, I could extract the io object out of the Conn to the dispatcher(this was ~2 months ago). But the fact that the pool knows it is only as a KeepAlive was a big hurdle. I had to remove the pool or rewrite it. Eventually I gave up.

[seanmonstar]

A solution for #1449 was just merged to master, that introduces a lower-level connection API, where it is possible to make CONNECT and upgrade requests on it.

The Client uses it internally, but still specifically errors on CONNECT and upgrades since there is no exposed way to handle them correctly.

[seanmonstar]

There's a proposed PR at #1563 to add this feature (and for servers).