-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support body write interruption #3121
Comments
I think I get what you mean. That if the IO transport currently doesn't want any more bytes written (or the HTTP/2 stream), then hyper won't be call Does that sound right? |
Yep! |
Seems like a good thing to solve. Would you like to propose a solution and/or alternatives? Or anyone else is free to as well. |
The option I mentioned above could look something like pub trait Body {
// existing API
/// Determine if the body is still in a healthy state without polling for the next frame.
///
/// `Body` consumers can use this method to check if the body has entered an error state even when the consumer is not
/// yet ready to try to read the next frame from the body. Since healthiness is not an operation that completes, this
/// method returns just a `Result<...>` rather than a `Poll<...>`.
///
/// For example, a `Body` implementation could maintain a timer counting down between `poll_frame` calls and report an
/// error from `poll_healthy` when time expires.
///
/// The default implementation simply returns `Ok(())`.
fn poll_healthy(
self: Pin<&mut Self>,
cx: &mut Context<'_>,
) -> Result<(), Self::Error> {
Ok(())
}
} The nice thing about this is that it enables A more limited option could be to allow a frame timeout to be configured but that's inherently going to limit how much control user code has over how it wants to manage this kind of cancellation. |
Ping @davidpdrsn @LucioFranco, anyone else who should see this issue? |
I put together an implementation of this. |
I think this is a really interesting and good idea, though the API design having it be a poll fn that doesn't return |
Yeah, should we rename it to |
So if I understand correctly, the idea is that generally timeout works fine if the consumer of the body (user of hyper) is polling it but that timer will never get triggered if they don't poll the body. So this is a way to allow hyper to figure out if the body is good or not without initiation a frame read from socket? So this api will never be used by consumers but basically only by hypers connection logic etc?
Is there any precedent on fn that take context but are not named poll? I think Also thanks for that example in hyperium/http-body#90 (comment) it was very helpful. |
Generally it'd only be used by Hyper. In the future we could implement it for Hyper's own body type so consumers could do the same kind of error short-circuiting, but that's not really the use case I'm interested in. I'm not aware off the top of my head any non-poll methods taking the context. |
If a peer stops writing the body of a request or response, a Hyper client or server has the ability to detect that and handle it however it likes. However, the same is not true if the peer stops reading the body of the request or response that Hyper is sending to it - the write will just indefinitely hang waiting on space in the stream.
There should be some way for consumers to tell Hyper to abort the write. For example, there could be a
poll_abort
method onBody
which Hyper will poll even when it can't request another body chunk. The body implementation could maintain whatever timeout logic it wants internally and ensure the task is woken up at the appropriate time.The text was updated successfully, but these errors were encountered: