diff --git a/examples/src/http/example-http-client.rs b/examples/src/http/example-http-client.rs index 9e2f5438..87a7af9f 100644 --- a/examples/src/http/example-http-client.rs +++ b/examples/src/http/example-http-client.rs @@ -44,8 +44,7 @@ async fn main() -> Result<(), BoxError> { .callee_name("example.http.server") // set default target address .address("127.0.0.1:8080".parse::().unwrap()) - .header("Test", "Test")? - .fail_on_error_status(true); + .header("Test", "Test")?; builder.build() }; diff --git a/volo-http/src/client/layer.rs b/volo-http/src/client/layer.rs new file mode 100644 index 00000000..74c317c2 --- /dev/null +++ b/volo-http/src/client/layer.rs @@ -0,0 +1,242 @@ +//! Collections of some useful `Layer`s. + +use std::{error::Error, fmt, time::Duration}; + +use http::status::StatusCode; +use motore::{layer::Layer, service::Service}; + +use crate::{ + error::{client::request_error, ClientError}, + response::ClientResponse, +}; + +/// [`Layer`] for setting timeout to the request. +/// +/// See [`TimeoutLayer::new`] for more details. +pub struct TimeoutLayer { + duration: Duration, +} + +impl TimeoutLayer { + /// Create a new [`TimeoutLayer`] with given [`Duration`]. + /// + /// If the request times out, an error [`Timeout`] is returned. + /// + /// [`Timeout`]: crate::error::client::Timeout + pub fn new(duration: Duration) -> Self { + Self { duration } + } +} + +impl Layer for TimeoutLayer { + type Service = TimeoutService; + + fn layer(self, inner: S) -> Self::Service { + TimeoutService { + inner, + duration: self.duration, + } + } +} + +/// The [`Service`] generated by [`TimeoutLayer`]. +/// +/// See [`TimeoutLayer`] and [`TimeoutLayer::new`] for more details. +pub struct TimeoutService { + inner: S, + duration: Duration, +} + +impl Service for TimeoutService +where + Cx: Send, + Req: Send, + S: Service + Send + Sync, +{ + type Response = S::Response; + type Error = S::Error; + + async fn call(&self, cx: &mut Cx, req: Req) -> Result { + let fut = self.inner.call(cx, req); + let sleep = tokio::time::sleep(self.duration); + + tokio::select! { + res = fut => res, + _ = sleep => { + tracing::error!("[Volo-HTTP] request timeout"); + Err(crate::error::client::timeout()) + } + } + } +} + +/// [`Layer`] for throwing service error with the response's error status code. +/// +/// Users can use [`FailOnStatus::all`], [`FailOnStatus::client_error`] or +/// [`FailOnStatus::server_error`] for creating the [`FailOnStatus`] layer that convert all (4XX and +/// 5XX), client error (4XX) or server error (5XX) to a error of service. +pub struct FailOnStatus { + client_error: bool, + server_error: bool, +} + +impl FailOnStatus { + /// Create a [`FailOnStatus`] layer that return error [`StatusCodeError`] for all error status + /// codes (4XX and 5XX). + pub fn all() -> Self { + Self { + client_error: true, + server_error: true, + } + } + + /// Create a [`FailOnStatus`] layer that return error [`StatusCodeError`] for client error + /// status codes (4XX). + pub fn client_error() -> Self { + Self { + client_error: true, + server_error: false, + } + } + + /// Create a [`FailOnStatus`] layer that return error [`StatusCodeError`] for server error + /// status codes (5XX). + pub fn server_error() -> Self { + Self { + client_error: false, + server_error: true, + } + } +} + +impl Layer for FailOnStatus { + type Service = FailOnStatusService; + + fn layer(self, inner: S) -> Self::Service { + FailOnStatusService { + inner, + fail_on: self, + } + } +} + +/// The [`Service`] generated by [`FailOnStatus`] layer. +/// +/// See [`FailOnStatus`] for more details. +pub struct FailOnStatusService { + inner: S, + fail_on: FailOnStatus, +} + +impl Service for FailOnStatusService +where + Cx: Send, + Req: Send, + S: Service, Error = ClientError> + Send + Sync, +{ + type Response = S::Response; + type Error = S::Error; + + async fn call(&self, cx: &mut Cx, req: Req) -> Result { + let resp = self.inner.call(cx, req).await?; + let status = resp.status(); + if (self.fail_on.client_error && status.is_client_error()) + || (self.fail_on.server_error && status.is_server_error()) + { + Err(request_error(StatusCodeError::new(status))) + } else { + Ok(resp) + } + } +} + +/// Client received a response with an error status code. +pub struct StatusCodeError { + /// The original status code + pub status: StatusCode, +} + +impl StatusCodeError { + fn new(status: StatusCode) -> Self { + Self { status } + } +} + +impl fmt::Debug for StatusCodeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("StatusCodeError") + .field("status", &self.status) + .finish() + } +} + +impl fmt::Display for StatusCodeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "client received an error status code: {}", self.status) + } +} + +impl Error for StatusCodeError {} + +#[cfg(test)] +mod client_layers_tests { + use http::status::StatusCode; + use motore::service::Service; + + use super::FailOnStatus; + use crate::{ + body::Body, client::test_helpers::MockTransport, context::ClientContext, + error::ClientError, request::ClientRequest, response::ClientResponse, ClientBuilder, + }; + + struct ReturnStatus; + + impl Service for ReturnStatus { + type Response = ClientResponse; + type Error = ClientError; + + fn call( + &self, + _: &mut ClientContext, + req: ClientRequest, + ) -> impl std::future::Future> + Send { + let path = req.uri().path(); + assert_eq!(&path[..1], "/"); + let status_code = path[1..].parse::().expect("invalid uri"); + let status_code = StatusCode::from_u16(status_code).expect("invalid status code"); + let mut resp = ClientResponse::new(Body::empty()); + *resp.status_mut() = status_code; + async { Ok(resp) } + } + } + + #[tokio::test] + async fn fail_on_status_test() { + { + // Reject all error status codes + let client = ClientBuilder::new() + .layer_outer_front(FailOnStatus::all()) + .mock(MockTransport::service(ReturnStatus)); + client.get("/400").send().await.unwrap_err(); + client.get("/500").send().await.unwrap_err(); + } + { + // Reject client error status codes + let client = ClientBuilder::new() + .layer_outer_front(FailOnStatus::client_error()) + .mock(MockTransport::service(ReturnStatus)); + client.get("/400").send().await.unwrap_err(); + // 5XX is server error, it should not be handled + client.get("/500").send().await.unwrap(); + } + { + // Reject all error status codes + let client = ClientBuilder::new() + .layer_outer_front(FailOnStatus::server_error()) + .mock(MockTransport::service(ReturnStatus)); + // 4XX is client error, it should not be handled + client.get("/400").send().await.unwrap(); + client.get("/500").send().await.unwrap_err(); + } + } +} diff --git a/volo-http/src/client/meta.rs b/volo-http/src/client/meta.rs deleted file mode 100644 index 6babaef6..00000000 --- a/volo-http/src/client/meta.rs +++ /dev/null @@ -1,72 +0,0 @@ -use std::error::Error; - -use http_body::Body; -use motore::service::Service; -use volo::context::Context; - -use crate::{ - context::ClientContext, - error::client::{status_error, ClientError}, - request::ClientRequest, - response::ClientResponse, -}; - -#[derive(Clone)] -pub struct MetaService { - inner: S, -} - -impl MetaService { - pub(super) fn new(inner: S) -> Self { - Self { inner } - } -} - -impl Service> for MetaService -where - S: Service, Response = ClientResponse, Error = ClientError> - + Send - + Sync - + 'static, - B: Body + Send + 'static, - B::Data: Send, - B::Error: Into> + 'static, -{ - type Response = S::Response; - type Error = S::Error; - - async fn call( - &self, - cx: &mut ClientContext, - req: ClientRequest, - ) -> Result { - let config = cx.rpc_info().config().to_owned(); - let fut = self.inner.call(cx, req); - let res = match config.timeout { - Some(duration) => { - let sleep = tokio::time::sleep(duration); - tokio::select! { - res = fut => res, - _ = sleep => { - tracing::error!("[Volo-HTTP]: request timeout."); - return Err(crate::error::client::timeout()); - } - } - } - None => fut.await, - }; - - if !config.fail_on_error_status { - return res; - } - - let resp = res?; - - let status = resp.status(); - if status.is_client_error() || status.is_server_error() { - Err(status_error(status)) - } else { - Ok(resp) - } - } -} diff --git a/volo-http/src/client/mod.rs b/volo-http/src/client/mod.rs index 8e8f2c05..7ea0e140 100644 --- a/volo-http/src/client/mod.rs +++ b/volo-http/src/client/mod.rs @@ -33,12 +33,11 @@ use self::{ callopt::CallOpt, dns::parse_target, loadbalance::{DefaultLB, DefaultLBService, LbConfig}, - meta::MetaService, target::TargetParser, transport::{ClientConfig, ClientTransport, ClientTransportConfig}, }; use crate::{ - context::{client::Config, ClientContext}, + context::ClientContext, error::{ client::{builder_error, no_address, ClientError, Result}, BoxError, @@ -51,8 +50,8 @@ pub mod callopt; #[cfg(feature = "cookie")] pub mod cookie; pub mod dns; +pub mod layer; pub mod loadbalance; -mod meta; mod request_builder; pub mod target; #[cfg(test)] @@ -69,7 +68,7 @@ pub mod prelude { const PKG_NAME_WITH_VER: &str = concat!(env!("CARGO_PKG_NAME"), '/', env!("CARGO_PKG_VERSION")); /// Default inner service of [`Client`] -pub type ClientMetaService = MetaService; +pub type ClientMetaService = ClientTransport; /// Default [`Client`] without any extra [`Layer`]s pub type DefaultClient = Client<
    >::Service>>>::Service>; @@ -98,9 +97,7 @@ pub struct ClientBuilder { /// This is unstable now and may be changed in the future. #[doc(hidden)] pub struct BuilderConfig { - pub timeout: Option, pub stat_enable: bool, - pub fail_on_error_status: bool, #[cfg(feature = "__tls")] pub disable_tls: bool, } @@ -108,9 +105,7 @@ pub struct BuilderConfig { impl Default for BuilderConfig { fn default() -> Self { Self { - timeout: None, stat_enable: true, - fail_on_error_status: false, #[cfg(feature = "__tls")] disable_tls: false, } @@ -547,14 +542,6 @@ impl ClientBuilder { self } - /// Return `Err` rather than full `ClientResponse` when the response status code is 4xx or 5xx. - /// - /// Default is false. - pub fn fail_on_error_status(&mut self, fail_on_error_status: bool) -> &mut Self { - self.builder_config.fail_on_error_status = fail_on_error_status; - self - } - /// Disable TLS for the client. /// /// Default is false, when TLS related feature is enabled, TLS is enabled by default. @@ -626,19 +613,10 @@ impl ClientBuilder { 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); - self - } - /// Build the HTTP client. pub fn build(mut self) -> C::Target where - IL: Layer>, + IL: Layer, IL::Service: Send + Sync + 'static, LB: MkLbLayer, LB::Layer: Layer, @@ -659,7 +637,7 @@ impl ClientBuilder { #[cfg(feature = "__tls")] self.tls_config.unwrap_or_default(), ); - let meta_service = MetaService::new(transport); + let meta_service = transport; let service = self.outer_layer.layer( self.mk_lb .make() @@ -677,17 +655,12 @@ impl ClientBuilder { HeaderValue::from_str(caller_name.as_str()).expect("Invalid caller name"), ); } - let config = Config { - timeout: self.builder_config.timeout, - fail_on_error_status: self.builder_config.fail_on_error_status, - }; let client_inner = ClientInner { service, caller_name, callee_name: self.callee_name, default_target: self.target, - default_config: config, default_call_opt: self.call_opt, target_parser: self.target_parser, headers: self.headers, @@ -704,7 +677,6 @@ struct ClientInner { caller_name: FastStr, callee_name: FastStr, default_target: Target, - default_config: Config, default_call_opt: Option, target_parser: TargetParser, headers: HeaderMap, @@ -826,7 +798,6 @@ impl Client { /// .uri("/") /// .body(Body::empty()) /// .expect("build request failed"), - /// None, /// ) /// .await /// .expect("request failed") @@ -841,7 +812,6 @@ impl Client { target: Target, call_opt: Option, mut request: ClientRequest, - timeout: Option, ) -> Result where S: Service, Response = ClientResponse, Error = ClientError> @@ -893,10 +863,6 @@ impl Client { cx.rpc_info_mut().callee_mut().set_service_name(callee_name); (self.inner.target_parser)(target, call_opt, cx.rpc_info_mut().callee_mut()); - let config = cx.rpc_info_mut().config_mut(); - config.clone_from(&self.inner.default_config); - config.timeout = timeout.or(config.timeout); - self.call(&mut cx, request).await } } @@ -974,10 +940,7 @@ mod client_tests { }; #[cfg(feature = "cookie")] use crate::client::cookie::CookieLayer; - use crate::{ - body::BodyConversion, error::client::status_error, utils::consts::HTTP_DEFAULT_PORT, - ClientBuilder, - }; + use crate::{body::BodyConversion, utils::consts::HTTP_DEFAULT_PORT, ClientBuilder}; #[derive(Deserialize)] struct HttpBinResponse { @@ -1170,24 +1133,6 @@ mod client_tests { assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } - #[tokio::test] - async fn fail_on_status() { - let mut builder = Client::builder(); - builder.host("httpbin.org").fail_on_error_status(true); - let client = builder.build(); - assert_eq!( - format!( - "{}", - client - .get("/post") - .send() - .await - .expect_err("GET for httpbin.org/post should fail") - ), - format!("{}", status_error(StatusCode::METHOD_NOT_ALLOWED)), - ); - } - #[cfg(feature = "__tls")] #[tokio::test] async fn client_disable_tls() { diff --git a/volo-http/src/client/request_builder.rs b/volo-http/src/client/request_builder.rs index 41d58293..12bfb015 100644 --- a/volo-http/src/client/request_builder.rs +++ b/volo-http/src/client/request_builder.rs @@ -2,7 +2,7 @@ //! //! See [`RequestBuilder`] for more details. -use std::{error::Error, time::Duration}; +use std::error::Error; use http::{ header::{HeaderMap, HeaderName, HeaderValue}, @@ -30,7 +30,6 @@ pub struct RequestBuilder { target: Target, call_opt: Option, request: Result>, - timeout: Option, } impl RequestBuilder { @@ -40,7 +39,6 @@ impl RequestBuilder { target: Default::default(), call_opt: Default::default(), request: Ok(ClientRequest::default()), - timeout: None, } } @@ -412,7 +410,6 @@ impl RequestBuilder { target: self.target, call_opt: self.call_opt, request, - timeout: self.timeout, } } @@ -420,15 +417,6 @@ impl RequestBuilder { pub fn body_ref(&self) -> Option<&B> { self.request.as_ref().ok().map(Request::body) } - - /// Set 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) -> Self { - self.timeout = Some(timeout); - self - } } impl RequestBuilder @@ -442,7 +430,7 @@ where /// Send the request and get the response. pub async fn send(self) -> Result { self.client - .send_request(self.target, self.call_opt, self.request?, self.timeout) + .send_request(self.target, self.call_opt, self.request?) .await } } diff --git a/volo-http/src/client/test_helpers.rs b/volo-http/src/client/test_helpers.rs index f53992d2..74e19856 100644 --- a/volo-http/src/client/test_helpers.rs +++ b/volo-http/src/client/test_helpers.rs @@ -10,20 +10,14 @@ use motore::{ }; use volo::{client::MkClient, context::Endpoint}; -use super::{ - callopt::CallOpt, meta::MetaService, Client, ClientBuilder, ClientInner, Target, - PKG_NAME_WITH_VER, -}; +use super::{callopt::CallOpt, Client, ClientBuilder, ClientInner, Target, PKG_NAME_WITH_VER}; use crate::{ - context::client::{ClientContext, Config}, - error::ClientError, - request::ClientRequest, - response::ClientResponse, - utils::test_helpers::mock_address, + context::client::ClientContext, error::ClientError, request::ClientRequest, + response::ClientResponse, utils::test_helpers::mock_address, }; /// Default mock service of [`Client`] -pub type ClientMockService = MetaService; +pub type ClientMockService = MockTransport; /// Default [`Client`] without any extra [`Layer`]s pub type DefaultMockClient = Client<
      >::Service>>::Service>; @@ -118,14 +112,14 @@ impl ClientBuilder { /// Build a mock HTTP client with a [`MockTransport`] service. pub fn mock(mut self, transport: MockTransport) -> C::Target where - IL: Layer>, + IL: Layer, IL::Service: Send + Sync + 'static, // remove loadbalance here OL: Layer, OL::Service: Send + Sync + 'static, C: MkClient>, { - let meta_service = MetaService::new(transport); + let meta_service = transport; let service = self.outer_layer.layer(self.inner_layer.layer(meta_service)); let caller_name = if self.caller_name.is_empty() { @@ -139,10 +133,6 @@ impl ClientBuilder { HeaderValue::from_str(caller_name.as_str()).expect("Invalid caller name"), ); } - let config = Config { - timeout: self.builder_config.timeout, - fail_on_error_status: self.builder_config.fail_on_error_status, - }; let client_inner = ClientInner { service, @@ -150,7 +140,6 @@ impl ClientBuilder { callee_name: self.callee_name, // set a default target so that we can create a request without authority default_target: Target::from_address(mock_address()), - default_config: config, default_call_opt: self.call_opt, // do nothing target_parser: parse_target, @@ -230,24 +219,4 @@ mod mock_transport_tests { assert!(resp.headers().is_empty()); assert!(resp.into_body().into_vec().await.unwrap().is_empty()); } - - #[tokio::test] - async fn status_response_test() { - { - let client = - ClientBuilder::new().mock(MockTransport::status_code(StatusCode::IM_A_TEAPOT)); - let resp = client.get("/").send().await.unwrap(); - assert_eq!(resp.status(), StatusCode::IM_A_TEAPOT); - assert!(resp.headers().is_empty()); - assert!(resp.into_body().into_vec().await.unwrap().is_empty()); - } - { - let client = { - let mut builder = ClientBuilder::new(); - builder.fail_on_error_status(true); - builder.mock(MockTransport::status_code(StatusCode::IM_A_TEAPOT)) - }; - assert!(client.get("/").send().await.is_err()); - } - } } diff --git a/volo-http/src/context/client.rs b/volo-http/src/context/client.rs index e5d90adc..4a095b58 100644 --- a/volo-http/src/context/client.rs +++ b/volo-http/src/context/client.rs @@ -1,7 +1,5 @@ //! Context and its utilities of client -use std::time::Duration; - use chrono::{DateTime, Local}; use volo::{ context::{Reusable, Role, RpcCx, RpcInfo}, @@ -61,15 +59,8 @@ impl ClientStats { /// Configuration of the request #[derive(Clone, Debug, Default)] -pub struct Config { - /// Timeout of the whole request - pub timeout: Option, - /// Return `Err` if status code of response is 4XX or 5XX - pub fail_on_error_status: bool, -} +pub struct Config; impl Reusable for Config { - fn clear(&mut self) { - *self = Default::default() - } + fn clear(&mut self) {} } diff --git a/volo-http/src/error/client.rs b/volo-http/src/error/client.rs index f3241e40..f8cda040 100644 --- a/volo-http/src/error/client.rs +++ b/volo-http/src/error/client.rs @@ -2,7 +2,7 @@ use std::{error::Error, fmt}; -use http::{StatusCode, Uri}; +use http::uri::Uri; use paste::paste; use super::BoxError; @@ -99,13 +99,6 @@ pub enum ErrorKind { /// [LoadBalance]: volo::loadbalance::LoadBalance /// [Discover]: volo::discovery::Discover LoadBalance, - /// Client received a response with a 4XX or 5XX status code - /// - /// This error will only be returned when - /// [`ClientBuilder::fail_on_error_status`][fail_on_error_status] enabled. - /// - /// [fail_on_error_status]: crate::client::ClientBuilder::fail_on_error_status - Status(StatusCode), /// Something wrong when processing on [`Body`](crate::body::Body) Body, } @@ -142,11 +135,6 @@ where ClientError::new(ErrorKind::LoadBalance, Some(error)) } -/// Create a [`ClientError`] with [`ErrorKind::Status`] -pub fn status_error(status: StatusCode) -> ClientError { - ClientError::new(ErrorKind::Status(status), None::) -} - impl From for ClientError { fn from(value: BodyConvertError) -> Self { ClientError::new(ErrorKind::Body, Some(BoxError::from(value))) @@ -160,14 +148,6 @@ impl std::fmt::Display for ErrorKind { Self::Context => f.write_str("processing context error"), Self::Request => f.write_str("sending request error"), Self::LoadBalance => f.write_str("load balance error"), - Self::Status(ref status) => { - let prefix = if status.is_client_error() { - "HTTP status client error" - } else { - "HTTP status server error" - }; - write!(f, "{prefix} ({status})") - } Self::Body => f.write_str("processing body error"), } } diff --git a/volo-http/src/server/layer/mod.rs b/volo-http/src/server/layer/mod.rs index 88c0cd8f..6b5d22b0 100644 --- a/volo-http/src/server/layer/mod.rs +++ b/volo-http/src/server/layer/mod.rs @@ -1,8 +1,6 @@ //! Collections of some useful `Layer`s. -//! -//! See [`FilterLayer`] and [`TimeoutLayer`] for more details. -pub(crate) mod body_limit; +mod body_limit; mod filter; mod timeout;