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

feat(body): add trailers to Body channel (#2260) #2387

Merged
merged 8 commits into from Jan 15, 2021
3 changes: 1 addition & 2 deletions benches/end_to_end.rs
Expand Up @@ -8,7 +8,7 @@ use std::net::SocketAddr;
use futures_util::future::join_all;

use hyper::client::HttpConnector;
use hyper::{body::HttpBody as _, Body, HeaderMap, Method, Request, Response, Server};
use hyper::{body::HttpBody as _, Body, Method, Request, Response, Server};

// HTTP1

Expand Down Expand Up @@ -313,7 +313,6 @@ impl Opts {
for _ in 0..chunk_cnt {
tx.send_data(chunk.into()).await.expect("send_data");
}
tx.send_trailers(HeaderMap::new()).expect("send_trailers");
});
body
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/body/body.rs
Expand Up @@ -382,7 +382,7 @@ impl HttpBody for Body {
..
} => match ready!(Pin::new(trailers_rx).poll(cx)) {
Ok(t) => Poll::Ready(Ok(Some(t))),
Err(_) => Poll::Ready(Err(crate::Error::new_closed())),
Err(_) => Poll::Ready(Ok(None)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seanmonstar If we don't send any trailers, polling the oneshot receiver returns Err(Canceled) so if we return Err(..) in this case, receive process will result in error. That's why I changed this to Ok(None). This way we don't have to send empty trailers. Is it ok though?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, this change is the correct behavior. Otherwise, when people upgrade hyper then suddenly in H2 when the trailers are polled they'd error the stream since they weren't sending any before.

},
#[cfg(feature = "ffi")]
Kind::Ffi(ref mut body) => body.poll_trailers(cx),
Expand Down Expand Up @@ -568,7 +568,7 @@ impl Sender {
}

/// Send trailers on trailers channel.
pub fn send_trailers(&mut self, trailers: HeaderMap) -> crate::Result<()> {
pub async fn send_trailers(&mut self, trailers: HeaderMap) -> crate::Result<()> {
let tx = match self.trailers_tx.take() {
Some(tx) => tx,
None => return Err(crate::Error::new_closed()),
Expand Down
5 changes: 1 addition & 4 deletions src/client/conn.rs
Expand Up @@ -63,10 +63,7 @@ use tower_service::Service;

use super::dispatch;
use crate::body::HttpBody;
use crate::common::{
exec::{BoxSendFuture, Exec},
task, Future, Pin, Poll,
};
use crate::common::{task, exec::{BoxSendFuture, Exec}, Future, Pin, Poll};
use crate::proto;
use crate::rt::Executor;
#[cfg(feature = "http1")]
Expand Down