From cf919de61ba2415d7f1362369985993cc967caf8 Mon Sep 17 00:00:00 2001 From: Joe Dahlquist Date: Sat, 29 Jan 2022 17:35:01 -0800 Subject: [PATCH 1/2] tonic: fix consistency issue with timeout func in transport/server builder transport/server/mod.rs exposes a Builder for the Server, and most of the Builder functions return Self (a change made for issue #115), but timeout returns mut& Self. This change makes timeout consistent with the rest of the Builder api by returning Self. Fixes: #900 Refs: #115 --- tonic/src/transport/server/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tonic/src/transport/server/mod.rs b/tonic/src/transport/server/mod.rs index 6a391cd62..a5bc884ab 100644 --- a/tonic/src/transport/server/mod.rs +++ b/tonic/src/transport/server/mod.rs @@ -191,12 +191,14 @@ impl Server { /// # use tonic::transport::Server; /// # use tower_service::Service; /// # use std::time::Duration; - /// # let mut builder = Server::builder(); + /// # let builder = Server::builder(); /// builder.timeout(Duration::from_secs(30)); /// ``` - pub fn timeout(&mut self, timeout: Duration) -> &mut Self { - self.timeout = Some(timeout); - self + pub fn timeout(self, timeout: Duration) -> Self { + Server { + timeout: Some(timeout), + ..self + } } /// Sets the [`SETTINGS_INITIAL_WINDOW_SIZE`][spec] option for HTTP2 From b95117d0e53c3976c9ca675d482bdca25d98a88f Mon Sep 17 00:00:00 2001 From: Joe Dahlquist Date: Wed, 16 Feb 2022 14:30:48 -0800 Subject: [PATCH 2/2] tonic: add must_use annotations to Server builder methods which return Self --- tonic/src/transport/server/mod.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tonic/src/transport/server/mod.rs b/tonic/src/transport/server/mod.rs index a5bc884ab..03f92eaf1 100644 --- a/tonic/src/transport/server/mod.rs +++ b/tonic/src/transport/server/mod.rs @@ -159,6 +159,7 @@ impl Server { /// Configure TLS for this server. #[cfg(feature = "tls")] #[cfg_attr(docsrs, doc(cfg(feature = "tls")))] + #[must_use] pub fn tls_config(self, tls_config: ServerTlsConfig) -> Result { Ok(Server { tls: Some(tls_config.tls_acceptor().map_err(Error::from_source)?), @@ -176,6 +177,7 @@ impl Server { /// # let builder = Server::builder(); /// builder.concurrency_limit_per_connection(32); /// ``` + #[must_use] pub fn concurrency_limit_per_connection(self, limit: usize) -> Self { Server { concurrency_limit: Some(limit), @@ -194,6 +196,7 @@ impl Server { /// # let builder = Server::builder(); /// builder.timeout(Duration::from_secs(30)); /// ``` + #[must_use] pub fn timeout(self, timeout: Duration) -> Self { Server { timeout: Some(timeout), @@ -207,6 +210,7 @@ impl Server { /// Default is 65,535 /// /// [spec]: https://http2.github.io/http2-spec/#SETTINGS_INITIAL_WINDOW_SIZE + #[must_use] pub fn initial_stream_window_size(self, sz: impl Into>) -> Self { Server { init_stream_window_size: sz.into(), @@ -217,6 +221,7 @@ impl Server { /// Sets the max connection-level flow control for HTTP2 /// /// Default is 65,535 + #[must_use] pub fn initial_connection_window_size(self, sz: impl Into>) -> Self { Server { init_connection_window_size: sz.into(), @@ -230,6 +235,7 @@ impl Server { /// Default is no limit (`None`). /// /// [spec]: https://http2.github.io/http2-spec/#SETTINGS_MAX_CONCURRENT_STREAMS + #[must_use] pub fn max_concurrent_streams(self, max: impl Into>) -> Self { Server { max_concurrent_streams: max.into(), @@ -246,6 +252,7 @@ impl Server { /// /// Default is no HTTP2 keepalive (`None`) /// + #[must_use] pub fn http2_keepalive_interval(self, http2_keepalive_interval: Option) -> Self { Server { http2_keepalive_interval, @@ -260,6 +267,7 @@ impl Server { /// /// Default is 20 seconds. /// + #[must_use] pub fn http2_keepalive_timeout(self, http2_keepalive_timeout: Option) -> Self { Server { http2_keepalive_timeout, @@ -275,6 +283,7 @@ impl Server { /// /// Default is no keepalive (`None`) /// + #[must_use] pub fn tcp_keepalive(self, tcp_keepalive: Option) -> Self { Server { tcp_keepalive, @@ -283,6 +292,7 @@ impl Server { } /// Set the value of `TCP_NODELAY` option for accepted connections. Enabled by default. + #[must_use] pub fn tcp_nodelay(self, enabled: bool) -> Self { Server { tcp_nodelay: enabled, @@ -295,6 +305,7 @@ impl Server { /// Passing `None` will do nothing. /// /// If not set, will default from underlying transport. + #[must_use] pub fn max_frame_size(self, frame_size: impl Into>) -> Self { Server { max_frame_size: frame_size.into(), @@ -310,6 +321,7 @@ impl Server { /// return confusing (but correct) protocol errors. /// /// Default is `false`. + #[must_use] pub fn accept_http1(self, accept_http1: bool) -> Self { Server { accept_http1, @@ -318,6 +330,7 @@ impl Server { } /// Intercept inbound headers and add a [`tracing::Span`] to each response future. + #[must_use] pub fn trace_fn(self, f: F) -> Self where F: Fn(&http::Request<()>) -> tracing::Span + Send + Sync + 'static,