hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.42k stars 1.59k forks source link

Feature request: send proto upgrade `Response` from handler. #3157

Open nurmohammed840 opened 1 year ago

nurmohammed840 commented 1 year ago

The upgrade example from 1.0.0-rc.3 looks odd (in my opinion).

In web socket context, If the server decides to upgrade the connection, It just send a 101 Switching Protocols response, and immediately use internal TCP as WebSocket protocol.

No need for extra task::spawn: https://github.com/hyperium/hyper/blob/9ed175d1545fa338b6212c4f48be9b3d12821c7a/examples/upgrades.rs#L58-L59

Sure, Its only possible, If there is an option to write response directly (rather then returning writable Response object)

Maybe add such API ?

For example:

async handle(req: Request<Incoming>) -> Result<Response<..> {
    match hyper::upgrade::send_response_and_upgrade(resp_body, req).await {
        Err(msg) => eprintln!("upgrade error: {msg}"),
        Ok(upgraded) => { ... }
    }
}
Noah-Kennedy commented 1 year ago

@nurmohammed840 why was this closed?

A248 commented 10 months ago

To elaborate on this issue. The serverside API design fits into hyper's model whereby a Service generates a response from a request; that is, a Service is more or less a Fn(Request) -> Response adorned with generics, futures, error handling.

Before a HTTP upgrade can happen, Hyper has to send the response to client indicating to do so. Thus, the client must receive the response before the server code can re-use the TCP stream. This necessitates that hyper::upgrade::on return a future which completes after the client has processed the request -- a future which necessarily completes after the function return, because if it completed before the function returned, you'd have access to the TCP stream before the upgrade had been negotiated and the HTTP protocol had been concluded.

I do agree, and I came here to write, that the current API deserves to be more flexible. The existing API heavily pushes users to spawn async tasks which entails unnecessary context switching and ownership movement. Working around the order-of-operations problem I described above is possible, and I've written code under the AGPL to do this.

The gains from more ergonomic HTTP upgrades allow callers to keep code organized according to the async model. Spawning a separate task introduces another code branch and thereby pre-empts traditional control flow. In many cases, as is the case here, the extra task is both unnecessary and introduces mental overhead. The framework of async/await was created with simple, sequential-looking, understandable control flow in mind. Hyper should support the sequential, task-free model to make the API and code using it easier to follow.