Open acfoltzer opened 3 years ago
For the record, this is also somewhat discussed in the issue about enabling Server Push (#1586), where I proposed something similar to hyper::upgrade
, and also pondered on whether it should also send 103 links if the version is HTTP/1.1.
Aha, I'd missed that thread because it didn't actually contain "hints". I like the look of that API as well; it would maintain the benefits of the channel approach without having to change the type signature of the service functions :+1: Perhaps it'd look something like this?
async fn my_service_fn(req: Request<Body>) -> Result<Response<Body>> {
let early_hints = Response::builder()
.status(103)
.header("Link", "</style.css>; rel=preload; as=style")
.header("Link", "</script.js>; rel=preload; as=script")
.body(())?;
hyper::interim_response(&mut req).send(early_hints).await?;
let resp = todo!("build the final response")?;
Ok(resp)
}
Yea, something like that. I think I favor that style as well, for the reasons you mentioned. Any thoughts on whether sending early hints should be considered combined with server push? I like the name hyper::push
more than interim_response
, :D.
I could see the argument that perhaps they should be separate, to provide more control (and it's possible Server Push may see less use as browsers deactivate it, with Early Hints as the suggested future even in h2 and h3).
Any thoughts on whether sending early hints should be considered combined with server push?
I don't have strongly held opinions about how the server push API should look, but I'd worry about confusion from exposing a single interface that can both push responses (presumably after sending a push promise elsewhere), and one that can only send 1xx responses with no associated promise. I could also see this confusion spilling over into implementation as you'd have to frame things differently in h2 for the two modes.
I've felt similar. I think it'd be clever if a simpler API degraded gracefully, but probably at hyper's level we shouldn't be too clever. That leaves the cleverness to server frameworks.
In case someone doesn't actually want to use this, we could consider having it a builder option. That way, if you're not going to send early hints, we don't construct a waste channel for each request.
Also, I've been thinking about the name. What about informational
?
In case someone doesn't actually want to use this, we could consider having it a builder option. That way, if you're not going to send early hints, we don't construct a waste channel for each request.
That sounds great. I certainly don't imagine it'll be the common case.
Also, I've been thinking about the name. What about
informational
?
Much better than my strawman :laughing:
I want to suggest a slightly different, fully parameter-based API. It might look something like this:
async fn my_service_fn(req: Request<Body>, mut responder: Responder) -> Result<()> {
let early_hints = Response::builder()
.status(103)
.header("Link", "</style.css>; rel=preload; as=style")
.header("Link", "</script.js>; rel=preload; as=script")
.body(())?;
responder.send_interim(early_hints).await?;
let resp = todo!("build the final response")?;
responder.send(resp).await?;
Ok(())
}
This API provides a more consistent interface than the previous suggested ones (apart from the stream one), since there's just one way through which responses can be sent.
Another advantage is that it would allow request handlers to easily perform work after the response has been sent. Currently in Hyper this can be achieved by spawning a task before returning from the function, but that needs 'static
and so isn't always viable. This could potentially reduce latency as destructors and other cleanup work can be run after the time-sensitive window between request and response is over.
Additionally this API could potentially support sending non-'static
bodies, which can reduce unnecessary allocation and copying.
Is there still no community consensus on the API surface? I am interested in experimenting with this.
Yep, this still needs an accepted proposal. I think the general leaning is to take the ideas for push and adapt them for informational
, and then run through any potential downsides with that design. The more clarity there is in the design proposal, the easier it would be to merge a PR afterwards.
Anyone is welcome to write up a proposal, of course!
@seanmonstar I am currently drafting a proposal gathering most of the proposed API ideas and evaluating their pros and cons, but the resulting Markdown document seems fairly large. Should I post it here as a comment or maybe open a new issue, PR or create a gist?
In the end, I decided to publish the resulting document as a gist: https://gist.github.com/vikanezrimaya/037101dc7b28de37ef03a47569213236
The gist above gathers most of the current ideas in one place and tries to evaluate them. Please note that while I touched Hyper before, it was merely as a user of the library (and also via the axum
framework), not as a contributor, so I might be wrong in some of my evaluations. If so, please do not hesitate to correct me.
Awesome, thanks for writing that up! I'll include some comments here:
Alternate idea: providing a "pusher" as a new argument
call(Req)
to call(Req, Pusher)
.Alternate idea: Sending a stream of responses
call() -> impl Future
to call() -> impl Stream
.futures-core
, but the proposal to add them to std
has the trait changing a fair bit (maybe AsyncIterator
, maybe the "keyword generics initiative").API based on the old Server Push proposal
I do think this is the best option. It's kinda like adding a "pusher" argument, it's just putting it inside the request extensions, and only if enabled. If for completeness sake you wanted to keep the others, I'd put them down in an appendix with a "can't use because X" (though it's also perfectly fine to drop them from the proposal entirely).
hyper::early_hints
I'd suggest a slightly more generic approach, not just Early Hints, but any informational (1xx) response. Could be hyper::informational
(or hyper::ext::informational
even), or another name that implies those kinds of responses.
Consumes the mutable reference to the request
That should be fine, it's just mutably borrowing the request to "take" the pusher out of the extensions. The request is still owned by the call
context.
@seanmonstar I have updated the document with your suggestions. Should we wait for more community feedback or should I potentially start a draft implementation to have some working code to play around with?
@acfoltzer did you want to take a look? This comment has a link to the gist.
Hey folks! Curious if there has been any movement/consensus around @vikanezrimaya proposal?
I am also interested in this. It would be really useful for something like resumable upload protocol: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-resumable-upload-02
An important customer of ours is very interested in using HTTP 103 Early Hints to lower the perceived latency experienced by their end users. This requires the ability to return more than one response head during the message exchange. The basic example from the RFC is:
With hyper's service function model, we currently only have the chance to return a single response head. Supporting multiple response heads may require some substantial API changes or additions, so I'm hoping we can discuss potential designs before diving into implementation. Please take any type signatures mentioned below to be a bit handwavy; I know things are more complex with the tower types and such.
Channel/stream-based interfaces
These are my preferred approaches from a hyper user's perspective, but I admit I tend to get lost when trying to follow all the types involved with tower services under the hood, so I'm not sure if this is particularly feasible on the implementation side.
Service function channel argument
We could add a variant of
hyper::service::service_fn()
whose closure takes an additional sender argument, i.e.,FnMut(Request<R>, mpsc::Sender<Response<()>>) -> S
. The service implementation would be free to send many interim responses, and would return the final response in the same way that existing service functions do. For example:Service function response stream return value
A similar variant is to accept a
Stream<Item = Result<Response<Body>>>
as the return value from a new service function variant. This would give the flexibility for the service function to use channels internally, or returnstream::once()
for a basic case with no hints. For example:Error handling gets a bit awkward with this one, as it always does when detaching a task from a service function. Also, we don't enforce that the 103 has no body via the type system as we do when the channel is dedicated only to returning interim responses.
Functional interface
Inspired by
hyper::upgrade::on()
, we could add a function (strawman namehyper::interim_response()
) that allows additional closures to be invoked with the current request, each of which would return an interim or final response. For example:Error handling is also awkward with this one, and there's a bit of a continuation-passing style feel, but it's worth considering something that resembles the existing 1xx API. I believe the other approaches would be easier to work with, particularly if multiple 103 responses are sent in a single exchange.
Extension to body types
I could imagine implementing subsequent response heads as something that could be polled from the body like trailers. This would probably run afoul of many of the same problems that motivated #2086, though, so it seems unlikely to be the right choice.
I'm sure I'm missing some ideas here, but regardless of the final design chosen it would be great to figure out a plan forward. Please let me know how we can help.