client(error): make display impl less verbose (#1283)

* client(error): make display impl less verbose * Update core/src/http_helpers.rs
paritytech · Feb 5, 2024 · 8f73dbe · 8f73dbe
1 parent 6975a79
commit 8f73dbe
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 79 deletions.
diff --git a/client/http-client/src/client.rs b/client/http-client/src/client.rs
@@ -275,9 +275,8 @@ impl<B, S> ClientT for HttpClient<S>
 where
 	S: Service<hyper::Request<Body>, Response = hyper::Response<B>, Error = TransportError> + Send + Sync + Clone,
 	<S as Service<hyper::Request<Body>>>::Future: Send,
-	B: HttpBody + Send + 'static,
+	B: HttpBody<Error = hyper::Error> + Send + 'static,
 	B::Data: Send,
-	B::Error: Into<Box<dyn StdError + Send + Sync>>,
 {
 	#[instrument(name = "notification", skip(self, params), level = "trace")]
 	async fn notification<Params>(&self, method: &str, params: Params) -> Result<(), Error>
@@ -408,9 +407,8 @@ impl<B, S> SubscriptionClientT for HttpClient<S>
 where
 	S: Service<hyper::Request<Body>, Response = hyper::Response<B>, Error = TransportError> + Send + Sync + Clone,
 	<S as Service<hyper::Request<Body>>>::Future: Send,
-	B: HttpBody + Send + 'static,
+	B: HttpBody<Error = hyper::Error> + Send + 'static,
 	B::Data: Send,
-	B::Error: Into<Box<dyn StdError + Send + Sync>>,
 {
 	/// Send a subscription request to the server. Not implemented for HTTP; will always return
 	/// [`Error::HttpNotImplemented`].

diff --git a/client/http-client/src/transport.rs b/client/http-client/src/transport.rs
@@ -11,7 +11,10 @@ use hyper::client::{Client, HttpConnector};
 use hyper::http::{HeaderMap, HeaderValue};
 use jsonrpsee_core::client::CertificateStore;
 use jsonrpsee_core::tracing::client::{rx_log_from_bytes, tx_log_from_str};
-use jsonrpsee_core::{http_helpers, GenericTransportError, TEN_MB_SIZE_BYTES};
+use jsonrpsee_core::{
+	http_helpers::{self, HttpError},
+	TEN_MB_SIZE_BYTES,
+};
 use std::error::Error as StdError;
 use std::future::Future;
 use std::pin::Pin;
@@ -45,7 +48,7 @@ impl Clone for HttpBackend {
 
 impl<B> tower::Service<hyper::Request<B>> for HttpBackend<B>
 where
-	B: HttpBody + Send + 'static,
+	B: HttpBody<Error = hyper::Error> + Send + 'static,
 	B::Data: Send,
 	B::Error: Into<Box<dyn StdError + Send + Sync>>,
 {
@@ -59,7 +62,7 @@ where
 			#[cfg(feature = "__tls")]
 			Self::Https(inner) => inner.poll_ready(ctx),
 		}
-		.map_err(Into::into)
+		.map_err(|e| Error::Http(e.into()))
 	}
 
 	fn call(&mut self, req: hyper::Request<B>) -> Self::Future {
@@ -69,7 +72,7 @@ where
 			Self::Https(inner) => inner.call(req),
 		};
 
-		Box::pin(async move { resp.await.map_err(Into::into) })
+		Box::pin(async move { resp.await.map_err(|e| Error::Http(e.into())) })
 	}
 }
 
@@ -279,9 +282,8 @@ pub struct HttpTransportClient<S> {
 impl<B, S> HttpTransportClient<S>
 where
 	S: Service<hyper::Request<Body>, Response = hyper::Response<B>, Error = Error> + Clone,
-	B: HttpBody + Send + 'static,
+	B: HttpBody<Error = hyper::Error> + Send + 'static,
 	B::Data: Send,
-	B::Error: Into<Box<dyn StdError + Send + Sync>>,
 {
 	async fn inner_send(&self, body: String) -> Result<hyper::Response<B>, Error> {
 		if body.len() > self.max_request_size as usize {
@@ -298,7 +300,7 @@ where
 		if response.status().is_success() {
 			Ok(response)
 		} else {
-			Err(Error::RequestFailure { status_code: response.status().into() })
+			Err(Error::Rejected { status_code: response.status().into() })
 		}
 	}
 
@@ -331,45 +333,25 @@ pub enum Error {
 	Url(String),
 
 	/// Error during the HTTP request, including networking errors and HTTP protocol errors.
-	#[error("HTTP error: {0}")]
-	Http(Box<dyn std::error::Error + Send + Sync>),
+	#[error("{0}")]
+	Http(#[from] HttpError),
 
 	/// Server returned a non-success status code.
-	#[error("Server returned an error status code: {:?}", status_code)]
-	RequestFailure {
-		/// Status code returned by the server.
+	#[error("Request rejected `{status_code}`")]
+	Rejected {
+		/// HTTP Status code returned by the server.
 		status_code: u16,
 	},
 
 	/// Request body too large.
 	#[error("The request body was too large")]
 	RequestTooLarge,
 
-	/// Malformed request.
-	#[error("Malformed request")]
-	Malformed,
-
 	/// Invalid certificate store.
 	#[error("Invalid certificate store")]
 	InvalidCertficateStore,
 }
 
-impl From<GenericTransportError> for Error {
-	fn from(err: GenericTransportError) -> Self {
-		match err {
-			GenericTransportError::TooLarge => Self::RequestTooLarge,
-			GenericTransportError::Malformed => Self::Malformed,
-			GenericTransportError::Inner(e) => Self::Http(e.into()),
-		}
-	}
-}
-
-impl From<hyper::Error> for Error {
-	fn from(err: hyper::Error) -> Self {
-		Self::Http(Box::new(err))
-	}
-}
-
 #[cfg(test)]
 mod tests {
 	use super::*;

diff --git a/client/transport/src/web.rs b/client/transport/src/web.rs
@@ -20,7 +20,7 @@ pub enum Error {
 	#[error("JS Error: {0:?}")]
 	Js(String),
 	/// WebSocket error
-	#[error("WebSocket Error: {0:?}")]
+	#[error("{0}")]
 	WebSocket(WebSocketError),
 	/// Operation not supported
 	#[error("Operation not supported")]

diff --git a/client/transport/src/ws/mod.rs b/client/transport/src/ws/mod.rs
@@ -187,7 +187,7 @@ pub enum WsHandshakeError {
 	Io(io::Error),
 
 	/// Error in the transport layer.
-	#[error("Error in the WebSocket handshake: {0}")]
+	#[error("{0}")]
 	Transport(#[source] soketto::handshake::Error),
 
 	/// Server rejected the handshake.
@@ -223,7 +223,7 @@ pub enum WsHandshakeError {
 #[derive(Debug, Error)]
 pub enum WsError {
 	/// Error in the WebSocket connection.
-	#[error("WebSocket connection error: {0}")]
+	#[error("{0}")]
 	Connection(#[source] soketto::connection::Error),
 	/// Message was too large.
 	#[error("The message was too large")]

diff --git a/core/src/client/error.rs b/core/src/client/error.rs
@@ -26,9 +26,9 @@
 
 //! Error type for client(s).
 
-use std::sync::Arc;
 use crate::{params::EmptyBatchRequest, RegisterMethodError};
 use jsonrpsee_types::{ErrorObjectOwned, InvalidRequestId};
+use std::sync::Arc;
 
 /// Error type.
 #[derive(Debug, thiserror::Error)]
@@ -37,10 +37,10 @@ pub enum Error {
 	#[error("{0}")]
 	Call(#[from] ErrorObjectOwned),
 	/// Networking error or error on the low-level protocol layer.
-	#[error("Networking or low-level protocol error: {0}")]
+	#[error("{0}")]
 	Transport(#[source] anyhow::Error),
 	/// The background task has been terminated.
-	#[error("The background task been terminated because: {0}; restart required")]
+	#[error("The background task closed {0}; restart required")]
 	RestartNeeded(Arc<Error>),
 	/// Failed to parse the data.
 	#[error("Parse error: {0}")]
@@ -55,7 +55,7 @@ pub enum Error {
 	#[error("Request timeout")]
 	RequestTimeout,
 	/// Max number of request slots exceeded.
-	#[error("Configured max number of request slots exceeded")]
+	#[error("Max concurrent requests exceeded")]
 	MaxSlotsExceeded,
 	/// Custom error.
 	#[error("Custom error: {0}")]
@@ -69,4 +69,4 @@ pub enum Error {
 	/// The error returned when registering a method or subscription failed.
 	#[error("{0}")]
 	RegisterMethod(#[from] RegisterMethodError),
-}
+}
diff --git a/core/src/error.rs b/core/src/error.rs
@@ -24,20 +24,6 @@
 // IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
-/// Generic transport error.
-#[derive(Debug, thiserror::Error)]
-pub enum GenericTransportError {
-	/// Request was too large.
-	#[error("The request was too big")]
-	TooLarge,
-	/// Malformed request
-	#[error("Malformed request")]
-	Malformed,
-	/// Concrete transport error.
-	#[error("Transport error: {0}")]
-	Inner(anyhow::Error),
-}
-
 /// A type that returns the error as a `String` from `SubscriptionCallback`.
 #[derive(Debug)]
 pub struct StringError(pub(crate) String);

diff --git a/core/src/http_helpers.rs b/core/src/http_helpers.rs
@@ -26,32 +26,38 @@
 
 //! Utility methods relying on hyper
 
-use crate::error::GenericTransportError;
-use anyhow::anyhow;
 use hyper::body::{Buf, HttpBody};
-use std::error::Error as StdError;
+
+/// Represents error that can when reading with a HTTP body.
+#[derive(Debug, thiserror::Error)]
+pub enum HttpError {
+	/// The HTTP message was too large.
+	#[error("The HTTP message was too big")]
+	TooLarge,
+	/// Malformed request
+	#[error("Malformed request")]
+	Malformed,
+	/// Represents error that can happen when dealing with HTTP streams.
+	#[error("{0}")]
+	Stream(#[from] hyper::Error),
+}
 
 /// Read a data from [`hyper::body::HttpBody`] and return the data if it is valid JSON and within the allowed size range.
 ///
 /// Returns `Ok((bytes, single))` if the body was in valid size range; and a bool indicating whether the JSON-RPC
 /// request is a single or a batch.
 /// Returns `Err` if the body was too large or the body couldn't be read.
-pub async fn read_body<B>(
-	headers: &hyper::HeaderMap,
-	body: B,
-	max_body_size: u32,
-) -> Result<(Vec<u8>, bool), GenericTransportError>
+pub async fn read_body<B>(headers: &hyper::HeaderMap, body: B, max_body_size: u32) -> Result<(Vec<u8>, bool), HttpError>
 where
-	B: HttpBody + Send + 'static,
+	B: HttpBody<Error = hyper::Error> + Send + 'static,
 	B::Data: Send,
-	B::Error: Into<Box<dyn StdError + Send + Sync>>,
 {
 	// NOTE(niklasad1): Values bigger than `u32::MAX` will be turned into zero here. This is unlikely to occur in
 	// practice and in that case we fallback to allocating in the while-loop below instead of pre-allocating.
 	let body_size = read_header_content_length(headers).unwrap_or(0);
 
 	if body_size > max_body_size {
-		return Err(GenericTransportError::TooLarge);
+		return Err(HttpError::TooLarge);
 	}
 
 	futures_util::pin_mut!(body);
@@ -61,7 +67,7 @@ where
 	let mut is_single = None;
 
 	while let Some(d) = body.data().await {
-		let data = d.map_err(|e| GenericTransportError::Inner(anyhow!(e.into())))?;
+		let data = d.map_err(HttpError::Stream)?;
 
 		// If it's the first chunk, trim the whitespaces to determine whether it's valid JSON-RPC call.
 		if received_data.is_empty() {
@@ -77,18 +83,18 @@ where
 					is_single = Some(false);
 					idx
 				}
-				_ => return Err(GenericTransportError::Malformed),
+				_ => return Err(HttpError::Malformed),
 			};
 
 			if data.chunk().len() - skip > max_body_size as usize {
-				return Err(GenericTransportError::TooLarge);
+				return Err(HttpError::TooLarge);
 			}
 
 			// ignore whitespace as these doesn't matter just makes the JSON decoding slower.
 			received_data.extend_from_slice(&data.chunk()[skip..]);
 		} else {
 			if data.chunk().len() + received_data.len() > max_body_size as usize {
-				return Err(GenericTransportError::TooLarge);
+				return Err(HttpError::TooLarge);
 			}
 
 			received_data.extend_from_slice(data.chunk());
@@ -104,7 +110,7 @@ where
 			);
 			Ok((received_data, single))
 		}
-		_ => Err(GenericTransportError::Malformed),
+		_ => Err(HttpError::Malformed),
 	}
 }
 

diff --git a/core/src/lib.rs b/core/src/lib.rs
@@ -60,7 +60,7 @@ cfg_client! {
 /// Shared tracing helpers to trace RPC calls.
 pub mod tracing;
 pub use async_trait::async_trait;
-pub use error::{GenericTransportError, RegisterMethodError, StringError};
+pub use error::{RegisterMethodError, StringError};
 
 /// JSON-RPC result.
 pub type RpcResult<T> = std::result::Result<T, jsonrpsee_types::ErrorObjectOwned>;

diff --git a/server/src/transport/http.rs b/server/src/transport/http.rs
@@ -1,5 +1,8 @@
 use http::Method;
-use jsonrpsee_core::{http_helpers::read_body, server::Methods, GenericTransportError};
+use jsonrpsee_core::{
+	http_helpers::{read_body, HttpError},
+	server::Methods,
+};
 
 use crate::{
 	middleware::rpc::{RpcService, RpcServiceBuilder, RpcServiceCfg, RpcServiceT},
@@ -77,9 +80,9 @@ where
 
 			let (body, is_single) = match read_body(&parts.headers, body, max_request_size).await {
 				Ok(r) => r,
-				Err(GenericTransportError::TooLarge) => return response::too_large(max_request_size),
-				Err(GenericTransportError::Malformed) => return response::malformed(),
-				Err(GenericTransportError::Inner(e)) => {
+				Err(HttpError::TooLarge) => return response::too_large(max_request_size),
+				Err(HttpError::Malformed) => return response::malformed(),
+				Err(HttpError::Stream(e)) => {
 					tracing::warn!(target: LOG_TARGET, "Internal error reading request body: {}", e);
 					return response::internal_error();
 				}