Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring websocket handling more inline with handler API #111

Closed
davidpdrsn opened this issue Aug 4, 2021 · 0 comments · Fixed by #121
Closed

Bring websocket handling more inline with handler API #111

davidpdrsn opened this issue Aug 4, 2021 · 0 comments · Fixed by #121
Labels
C-enhancement Category: A PR with an enhancement
Milestone

Comments

@davidpdrsn
Copy link
Member

Feature Request

Motivation

Currently handling websockets works quite differently from handling other requests. This has a few downsides:

  • You currently cannot return an error from a ws request. The connection will always be upgraded. This was also highlighted in WebSocketHandler::call should be allowed to fail #73.
  • You cannot customize the connection upgrade response. Users might wanna add additional headers. That is currently not possible.
  • Applying extractors to ws requests is possible but the implementation isn't very clear. It requires code very similar to what is already in handler/mod.rs.

Proposal

Change the websocket API to instead use a regular handler and a special IntoResponse. Something like

// build our application with a route
let app = route("/", get(handler));

async fn ws_handler(
    // this extractor checks for the required headers
    ws: extract::WebSocketUpgrade,
    // you can put other extractors here
) -> impl IntoResponse {
    // here you can do whatever you want to prepare for handling the connection
    // possibly return an error and rejection the connection all together

    // actually upgrade the connection and call the async closure with
    // the websocket connection when we have it
    ws.on_upgrade(|socket: WebSocket| async {
        // do stuff...
    })
}

This is similar to how things work in actix.

This solution was suggest by gbaranski#5119 on Discord.

Alternatives

Leads to slightly more code in the basic case where you don't wanna apply any extractors and just wanna start a ws connection without rejections. I think that tradeoff is worth it though.

@davidpdrsn davidpdrsn added the C-enhancement Category: A PR with an enhancement label Aug 4, 2021
@davidpdrsn davidpdrsn added this to the 0.2 milestone Aug 4, 2021
davidpdrsn added a commit that referenced this issue Aug 4, 2021
Fixes #111

Example usage:

```rust
use axum::{
    prelude::*,
    extract::ws::{WebSocketUpgrade, WebSocket},
    response::IntoResponse,
};

let app = route("/ws", get(handler));

async fn handler(ws: WebSocketUpgrade) -> impl IntoResponse {
    ws.on_upgrade(handle_socket)
}

async fn handle_socket(mut socket: WebSocket) {
    while let Some(msg) = socket.recv().await {
        let msg = if let Ok(msg) = msg {
            msg
        } else {
            // client disconnected
            return;
        };

        if socket.send(msg).await.is_err() {
            // client disconnected
            return;
        }
    }
}
```
davidpdrsn added a commit that referenced this issue Aug 4, 2021
Fixes #111

Example usage:

```rust
use axum::{
    prelude::*,
    extract::ws::{WebSocketUpgrade, WebSocket},
    response::IntoResponse,
};

let app = route("/ws", get(handler));

async fn handler(ws: WebSocketUpgrade) -> impl IntoResponse {
    ws.on_upgrade(handle_socket)
}

async fn handle_socket(mut socket: WebSocket) {
    while let Some(msg) = socket.recv().await {
        let msg = if let Ok(msg) = msg {
            msg
        } else {
            // client disconnected
            return;
        };

        if socket.send(msg).await.is_err() {
            // client disconnected
            return;
        }
    }
}
```
davidpdrsn added a commit that referenced this issue Aug 7, 2021
Fixes #111

Example usage:

```rust
use axum::{
    prelude::*,
    extract::ws::{WebSocketUpgrade, WebSocket},
    response::IntoResponse,
};

let app = route("/ws", get(handler));

async fn handler(ws: WebSocketUpgrade) -> impl IntoResponse {
    ws.on_upgrade(handle_socket)
}

async fn handle_socket(mut socket: WebSocket) {
    while let Some(msg) = socket.recv().await {
        let msg = if let Ok(msg) = msg {
            msg
        } else {
            // client disconnected
            return;
        };

        if socket.send(msg).await.is_err() {
            // client disconnected
            return;
        }
    }
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A PR with an enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant