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

use async fn in traits in SendStream #235

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ruben2424
Copy link
Contributor

@Ruben2424 Ruben2424 commented Apr 4, 2024

at the moment there are two trait functions to send data.

  • send_data to prepare data for sending
  • poll_ready to poll the stream until the data is sent

This PR uses the async fn in traits feature to introduce the async fn send_data function in the SendStream trait.

Advantages:

  • Simplifying the h3_quinn crate
    • Removes error paths. h3_quinn panics if some functions are called while the boxed future is not ready. Users of h3_quinn+ h3 might be able to get this, for example when using RequestStream in a Mutex. (but I'm not sure)
    • Avoids boxing the future
    • Avoids cloning the data before sending
  • In h3 it is no longer possible to accidentally call send_data without poll_ready

Disadvantages

  • Breaks MSRV
  • Send bounds? (maybe they can be removed).
  • idk maybe more?

Open:

  • Try to remove Send bounds
  • Fix WebTransport
  • Fix missing Tests
  • Documentation

Is it worth exploring this approach?

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

Successfully merging this pull request may close these issues.

1 participant