chore(volo-http): optimize user experience (#483)

This commit unified log format in `volo-http`, adjusted the timeout
interface and add generic types for `DefaultClient`.

Signed-off-by: Yu Li <[email protected]>

volo-http/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "volo-http"
-version = "0.2.11"
+version = "0.2.12"
 edition.workspace = true
 homepage.workspace = true
 repository.workspace = true

volo-http/src/client/loadbalance.rs

@@ -136,7 +136,7 @@ where
                     match channel.recv().await {
                         Ok(recv) => lb.rebalance(recv),
                         Err(err) => {
-                            tracing::warn!("[VOLO] discovering subscription error: {:?}", err)
+                            tracing::warn!("[Volo-HTTP] discovering subscription error: {:?}", err)
                         }
                     }
                 }

volo-http/src/client/mod.rs

@@ -67,7 +67,8 @@ const PKG_NAME_WITH_VER: &str = concat!(env!("CARGO_PKG_NAME"), '/', env!("CARGO
 /// Default inner service of [`Client`]
 pub type ClientMetaService = MetaService<ClientTransport>;
 /// Default [`Client`] without any extra [`Layer`]s
-pub type DefaultClient = Client<DefaultLBService<ClientMetaService>>;
+pub type DefaultClient<IL = Identity, OL = Identity> =
+    Client<<OL as Layer<DefaultLBService<<IL as Layer<ClientMetaService>>::Service>>>::Service>;
 
 /// A builder for configuring an HTTP [`Client`].
 pub struct ClientBuilder<IL, OL, C, LB> {
@@ -88,7 +89,11 @@ pub struct ClientBuilder<IL, OL, C, LB> {
     tls_config: Option<volo::net::tls::TlsConnector>,
 }
 
-struct BuilderConfig {
+/// Configuration for [`ClientBuilder`]
+///
+/// This is unstable now and may be changed in the future.
+#[doc(hidden)]
+pub struct BuilderConfig {
     timeout: Option<Duration>,
     stat_enable: bool,
     fail_on_error_status: bool,
@@ -511,16 +516,26 @@ impl<IL, OL, C, LB> ClientBuilder<IL, OL, C, LB> {
         &mut self.headers
     }
 
-    /// Get a reference to the HTTP configuration of the client.
-    pub fn http_config(&self) -> &ClientConfig {
+    /// Get a reference to HTTP configuration of the client.
+    pub fn http_config_ref(&self) -> &ClientConfig {
         &self.http_config
     }
 
-    /// Get a mutable reference to the HTTP configuration of the client.
+    /// Get a mutable reference to HTTP configuration of the client.
     pub fn http_config_mut(&mut self) -> &mut ClientConfig {
         &mut self.http_config
     }
 
+    /// Get a reference to builder configuration of the client.
+    pub fn builder_config_ref(&self) -> &BuilderConfig {
+        &self.builder_config
+    }
+
+    /// Get a mutable reference to builder configuration of the client.
+    pub fn builder_config_mut(&mut self) -> &mut BuilderConfig {
+        &mut self.builder_config
+    }
+
     /// This is unstable now and may be changed in the future.
     #[doc(hidden)]
     pub fn stat_enable(&mut self, enable: bool) -> &mut Self {
@@ -590,29 +605,29 @@ impl<IL, OL, C, LB> ClientBuilder<IL, OL, C, LB> {
     }
 
     /// Set the maximum idle time for a connection.
-    pub fn set_connect_timeout(&mut self, timeout: Duration) -> &mut Self {
-        self.connector.set_connect_timeout(Some(timeout));
+    pub fn set_connect_timeout(&mut self, timeout: Option<Duration>) -> &mut Self {
+        self.connector.set_connect_timeout(timeout);
         self
     }
 
     /// Set the maximum idle time for reading data from the connection.
-    pub fn set_read_timeout(&mut self, timeout: Duration) -> &mut Self {
-        self.connector.set_read_timeout(Some(timeout));
+    pub fn set_read_timeout(&mut self, timeout: Option<Duration>) -> &mut Self {
+        self.connector.set_read_timeout(timeout);
         self
     }
 
     /// Set the maximum idle time for writing data to the connection.
-    pub fn set_write_timeout(&mut self, timeout: Duration) -> &mut Self {
-        self.connector.set_write_timeout(Some(timeout));
+    pub fn set_write_timeout(&mut self, timeout: Option<Duration>) -> &mut Self {
+        self.connector.set_write_timeout(timeout);
         self
     }
 
     /// Set the maximin idle time for the request.
     ///
     /// The whole request includes connecting, writting, and reading the whole HTTP protocol
     /// headers (without reading response body).
-    pub fn set_request_timeout(&mut self, timeout: Duration) -> &mut Self {
-        self.builder_config.timeout = Some(timeout);
+    pub fn set_request_timeout(&mut self, timeout: Option<Duration>) -> &mut Self {
+        self.builder_config.timeout = timeout;
         self
     }
 
@@ -924,11 +939,12 @@ where
 mod client_tests {
     #![allow(unused)]
 
-    use std::collections::HashMap;
+    use std::{collections::HashMap, future::Future};
 
     use http::{header, StatusCode};
+    use motore::{layer::Layer, service::Service};
     use serde::Deserialize;
-    use volo::context::Endpoint;
+    use volo::{context::Endpoint, layer::Identity};
 
     use super::{
         callopt::CallOpt,
@@ -956,7 +972,46 @@ mod client_tests {
     const USER_AGENT_VAL: &str = "volo-http-unit-test";
 
     fn client_types_check() {
+        struct TestLayer;
+        struct TestService<S> {
+            inner: S,
+        }
+
+        impl<S> Layer<S> for TestLayer {
+            type Service = TestService<S>;
+
+            fn layer(self, inner: S) -> Self::Service {
+                TestService { inner }
+            }
+        }
+
+        impl<S, Cx, Req> Service<Cx, Req> for TestService<S>
+        where
+            S: Service<Cx, Req>,
+        {
+            type Response = S::Response;
+            type Error = S::Error;
+
+            fn call(
+                &self,
+                cx: &mut Cx,
+                req: Req,
+            ) -> impl Future<Output = Result<Self::Response, Self::Error>> + Send {
+                self.inner.call(cx, req)
+            }
+        }
+
         let _: DefaultClient = ClientBuilder::new().build();
+        let _: DefaultClient<TestLayer> = ClientBuilder::new().layer_inner(TestLayer).build();
+        let _: DefaultClient<TestLayer> = ClientBuilder::new().layer_inner_front(TestLayer).build();
+        let _: DefaultClient<Identity, TestLayer> =
+            ClientBuilder::new().layer_outer(TestLayer).build();
+        let _: DefaultClient<Identity, TestLayer> =
+            ClientBuilder::new().layer_outer_front(TestLayer).build();
+        let _: DefaultClient<TestLayer, TestLayer> = ClientBuilder::new()
+            .layer_inner(TestLayer)
+            .layer_outer(TestLayer)
+            .build();
     }
 
     #[tokio::test]

volo-http/src/client/transport.rs

@@ -94,7 +94,6 @@ impl ClientTransport {
             volo::net::conn::ConnStream::Tcp(tcp_stream) => tcp_stream,
             _ => unreachable!(),
         };
-        println!("target_name: {target_name}");
         self.tls_connector
             .connect(target_name, tcp_stream)
             .await

volo-http/src/server/layer.rs

@@ -121,7 +121,7 @@ where
             Ok(Err(res)) => Ok(res.into_response()),
             // something wrong while extracting
             Err(rej) => {
-                tracing::warn!("[VOLO] FilterLayer: something wrong while extracting");
+                tracing::warn!("[Volo-HTTP] FilterLayer: something wrong while extracting");
                 Ok(rej.into_response())
             }
         }

volo-http/src/server/mod.rs

@@ -24,7 +24,6 @@ use motore::{
 use parking_lot::RwLock;
 use scopeguard::defer;
 use tokio::sync::Notify;
-use tracing::{info, trace};
 #[cfg(feature = "__tls")]
 use volo::net::{conn::ConnStream, tls::Acceptor, tls::ServerTlsConfig};
 use volo::{
@@ -274,7 +273,7 @@ impl<S, L> Server<S, L> {
         let server = Arc::new(self.server);
         let service = Arc::new(self.layer.layer(self.service));
         let incoming = mk_incoming.make_incoming().await?;
-        info!("[VOLO] server start at: {:?}", incoming);
+        tracing::info!("[Volo-HTTP] server start at: {:?}", incoming);
 
         // count connections, used for graceful shutdown
         let conn_cnt = Arc::new(AtomicUsize::new(0));
@@ -322,15 +321,15 @@ impl<S, L> Server<S, L> {
         }
 
         if !self.shutdown_hooks.is_empty() {
-            info!("[VOLO] call shutdown hooks");
+            tracing::info!("[Volo-HTTP] call shutdown hooks");
 
             for hook in self.shutdown_hooks {
                 (hook)().await;
             }
         }
 
         // received signal, graceful shutdown now
-        info!("[VOLO] received signal, gracefully exiting now");
+        tracing::info!("[Volo-HTTP] received signal, gracefully exiting now");
         *exit_flag.write() = true;
 
         // Now we won't accept new connections.
@@ -345,9 +344,9 @@ impl<S, L> Server<S, L> {
             if conn_cnt.load(Ordering::Relaxed) == 0 {
                 break;
             }
-            trace!(
-                "[VOLO] gracefully exiting, remaining connection count: {}",
-                conn_cnt.load(Ordering::Relaxed)
+            tracing::trace!(
+                "[Volo-HTTP] gracefully exiting, remaining connection count: {}",
+                conn_cnt.load(Ordering::Relaxed),
             );
             tokio::time::sleep(Duration::from_secs(1)).await;
         }
@@ -389,7 +388,7 @@ async fn serve<I, S, E>(
                     let stream = match tls_config.acceptor.accept(stream).await {
                         Ok(conn) => conn,
                         Err(err) => {
-                            trace!("[VOLO] tls handshake error: {err:?}");
+                            tracing::trace!("[Volo-HTTP] tls handshake error: {err:?}");
                             continue;
                         }
                     };
@@ -401,11 +400,11 @@ async fn serve<I, S, E>(
 
         let peer = match conn.info.peer_addr {
             Some(ref peer) => {
-                trace!(" accept connection from: {peer:?}");
+                tracing::trace!("accept connection from: {peer:?}");
                 peer.clone()
             }
             None => {
-                info!("no peer address found from server connection");
+                tracing::info!("no peer address found from server connection");
                 continue;
             }
         };
@@ -449,20 +448,20 @@ async fn serve_conn<S>(
 
     tokio::select! {
         _ = &mut notified => {
-            tracing::trace!("[VOLO] closing a pending connection");
+            tracing::trace!("[Volo-HTTP] closing a pending connection");
             // Graceful shutdown.
             hyper::server::conn::http1::UpgradeableConnection::graceful_shutdown(
                 Pin::new(&mut http_conn)
             );
             // Continue to poll this connection until shutdown can finish.
             let result = http_conn.await;
             if let Err(err) = result {
-                tracing::debug!("[VOLO] connection error: {:?}", err);
+                tracing::debug!("[Volo-HTTP] connection error: {:?}", err);
             }
         }
         result = &mut http_conn => {
             if let Err(err) = result {
-                tracing::debug!("[VOLO] connection error: {:?}", err);
+                tracing::debug!("[Volo-HTTP] connection error: {:?}", err);
             }
         },
     }

volo-http/src/server/utils/serve_dir.rs

@@ -93,7 +93,7 @@ where
         let path = req.uri().path();
         let path = path.strip_prefix('/').unwrap_or(path);
 
-        tracing::trace!("ServeDir: path: {path}");
+        tracing::trace!("[Volo-HTTP] ServeDir: path: {path}");
 
         // Join to the serving directory and canonicalize it
         let path = self.path.join(path);
@@ -103,7 +103,7 @@ where
 
         // Reject file which is out of the serving directory
         if path.strip_prefix(self.path.as_path()).is_err() {
-            tracing::debug!("ServeDir: illegal path: {}", path.display());
+            tracing::debug!("[Volo-HTTP] ServeDir: illegal path: {}", path.display());
             return Ok(StatusCode::FORBIDDEN.into_response());
         }