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

Update tungstenite to 0.26 #3082

Closed
1 task done
coolreader18 opened this issue Dec 18, 2024 · 2 comments
Closed
1 task done

Update tungstenite to 0.26 #3082

coolreader18 opened this issue Dec 18, 2024 · 2 comments

Comments

@coolreader18
Copy link

  • I have looked for existing issues (including closed) about this

Feature Request

It'd be super nice to update to the latest version of [tokio-]tungstenite.

Motivation

The newest versions of tungstenite have Bytes as the payload type in Message instead of Vec<u8>, which lets you send shared data a client, e.g. sending the same message to N different clients without allocating N different buffers. Especially with axum 0.8 coming out as an API-breaking release, this would be super nice to have.

Proposal

Update to tungstenite 0.26 and replace Vec<u8> in axum::extract::ws::Message with Bytes.

The one tricky thing is that for Text, tungstenite defines a Utf8Bytes type, which is a wrapper over Bytes that guarantees utf8 validity (like String). There are a few different options for what type to use in axum's Message::Text variant, none of them obviously the right choice:

  • Use String, copying the Utf8Bytes into it.
    • This obviously isn't ideal, as it means users won't be able to get those benefits listed above if they're sending Text data. It is, however, the easiest in terms of API design.
  • Use tungstenite::Utf8Bytes.
    • This is probably a no-go, since axum meticulously wraps all of tungstenite's types, so it wouldn't make sense to make an exception just for this.
  • Make a new axum version of Utf8Bytes
    • This would mean a new API surface that's not actually websocket-specific, but actually a weirdly useful general utility for a utf8 Bytes wrapper. Definitely doable, but means there might be further feature requests for str versions of Bytes methods/Bytes versions of str methods, and also that the user will need to do conversions if they're already using a version of this type from something like bytestring or bytes-utils.
  • Use an existing version of this type, like bytestring::ByteString or bytes_utils::Str.
    • This adds a new public dependency, and both these crates are popular with roughly the same amount of downloads, meaning there's no obvious right choice.
  • Add a new type to the bytes crate and use that.
    • This is my personal pick if I had the say, but I'm not on the tokio dev team and it's obviously a much bigger task than any of the other ones. It'd require designing and adding tons of new API surface to a whole 'nother crate in the tokio ecosystem, which if this was to be worked on for axum 0.8 would delay it quite a bit. Maybe 0.9? 👀
@jplock
Copy link

jplock commented Dec 18, 2024

#3078

@jplatte jplatte closed this as completed Dec 18, 2024
@jplatte
Copy link
Member

jplatte commented Dec 18, 2024

Having Utf8Bytes upstream in bytes would be great indeed. I think the first step towards that would be looking for an existing issue or creating a new one if it doesn't exist. Please link it here if you do that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants