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 Hyper Client to support handling expect: 100-continue header #3833

Open
heiwais25 opened this issue Jan 17, 2025 · 3 comments
Open

Update Hyper Client to support handling expect: 100-continue header #3833

heiwais25 opened this issue Jan 17, 2025 · 3 comments
Labels
C-feature Category: feature. This is adding a new feature.

Comments

@heiwais25
Copy link

Is your feature request related to a problem? Please describe.

When I use Hyper Client based on the example (

hyper/examples/client.rs

Lines 49 to 54 in 3817a79

let (mut sender, conn) = hyper::client::conn::http1::handshake(io).await?;
tokio::task::spawn(async move {
if let Err(err) = conn.await {
println!("Connection failed: {:?}", err);
}
});
), it doesn't correctly handle the expect: 100-continue header in the request.

Describe the solution you'd like

Hyper Client needs to wait for the 100 continue response from the S3WSServer after sending the request header parts for certain time and then send the request body if it gets the response from the server or after certain time threshold.

This is to have the same functionality as https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/100

Describe alternatives you've considered

Expose the client parts where it sends the request header and body, but it will be inefficient for the developers to implement the same functionality.

Additional context
Add any other context or screenshots about the feature request here.

@heiwais25 heiwais25 added the C-feature Category: feature. This is adding a new feature. label Jan 17, 2025
@seanmonstar
Copy link
Member

I'm not certain I want to add explicit support inside hyper, because a timeout is needed, and I'd prefer as much as possible that those be done outside of hyper. That doesn't mean we couldn't eventually someday add explicit support, of course. But, in order to support it, it needs two things:

  1. Support for knowing when a 100 response is received (see Client support for receiving 1xx informational responses #2565)
  2. A body which combines waiting for that response or a timeout, whichever comes first.

I think we can provide both, the first one in hyper, and a helper body type in hyper-util which registers a callback and then provides a body wrapper that make use of it and a timmer.

@heiwais25
Copy link
Author

Thank you for sharing the earlier discussions and the follow up.

It would be great if the client supports this behavior somehow even though Hyper doesn't have explicit support.

I understand the point of on_information(), but have some questions about the second one.

It seems hyper-util uses SendRequest

pub fn try_send_request(
to send the request eventually which is in Hyper crate.

Could you share the high level idea about the second one without updating Hyper? I think we ended up updating Hyper.

@heiwais25
Copy link
Author

Also, to add few thoughts, I think it makes sense to add it to the Hyper explicitly since it is listed in RFC https://www.rfc-editor.org/rfc/rfc9110#name-expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

2 participants