From 588c144d132db2f62b138ac27d3fd78259ce40d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Thu, 10 Oct 2024 17:06:13 +0200 Subject: [PATCH 1/3] errors: expose downcast_ref for type-erased errors Sometimes, user might want to downcast `Arc` to specific error type. Actually, there is a use case for this in cpp-rust-driver, where we need to define custom `SerializeRow` implementation. It returns a custom error type. We would like to match against it during rust-to-C error conversion - downcasting is thus required. --- scylla-cql/src/types/deserialize/mod.rs | 10 ++++++++++ scylla-cql/src/types/serialize/mod.rs | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/scylla-cql/src/types/deserialize/mod.rs b/scylla-cql/src/types/deserialize/mod.rs index 639fc62553..acdd50c7a4 100644 --- a/scylla-cql/src/types/deserialize/mod.rs +++ b/scylla-cql/src/types/deserialize/mod.rs @@ -211,6 +211,11 @@ impl TypeCheckError { pub fn new(err: impl std::error::Error + Send + Sync + 'static) -> Self { Self(Arc::new(err)) } + + /// Retrieve an error reason by downcasting to specific type. + pub fn downcast_ref(&self) -> Option<&T> { + self.0.downcast_ref() + } } /// An error indicating that a failure happened during deserialization. @@ -236,6 +241,11 @@ impl DeserializationError { pub fn new(err: impl Error + Send + Sync + 'static) -> Self { Self(Arc::new(err)) } + + /// Retrieve an error reason by downcasting to specific type. + pub fn downcast_ref(&self) -> Option<&T> { + self.0.downcast_ref() + } } impl Display for DeserializationError { diff --git a/scylla-cql/src/types/serialize/mod.rs b/scylla-cql/src/types/serialize/mod.rs index b086dd5dcc..27c3c627b5 100644 --- a/scylla-cql/src/types/serialize/mod.rs +++ b/scylla-cql/src/types/serialize/mod.rs @@ -41,6 +41,11 @@ impl SerializationError { pub fn new(err: impl Error + Send + Sync + 'static) -> SerializationError { SerializationError(Arc::new(err)) } + + /// Retrieve an error reason by downcasting to specific type. + pub fn downcast_ref(&self) -> Option<&T> { + self.0.downcast_ref() + } } impl Display for SerializationError { From 42c4c658a30010e334b4e5b1a683ba837de4c61a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Thu, 10 Oct 2024 22:36:47 +0200 Subject: [PATCH 2/3] errors: downcast_ref for BrokenConnectionError BrokenConnectionError exposed a `get_reason` method, which returned an `Arc`. We decided it's better to provide our own implementation of `downcast_ref` for the type-erased error types. --- scylla/src/transport/connection.rs | 2 +- scylla/src/transport/errors.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scylla/src/transport/connection.rs b/scylla/src/transport/connection.rs index a7a4766379..1e2eaddac1 100644 --- a/scylla/src/transport/connection.rs +++ b/scylla/src/transport/connection.rs @@ -2780,7 +2780,7 @@ mod tests { let err = error_receiver.await.unwrap(); let err_inner: &BrokenConnectionErrorKind = match err { crate::transport::connection::ConnectionError::BrokenConnection(ref e) => { - e.get_reason().downcast_ref().unwrap() + e.downcast_ref().unwrap() } _ => panic!("Bad error type. Expected keepalive timeout."), }; diff --git a/scylla/src/transport/errors.rs b/scylla/src/transport/errors.rs index f6b31b6c95..ff0eb56ca6 100644 --- a/scylla/src/transport/errors.rs +++ b/scylla/src/transport/errors.rs @@ -728,15 +728,15 @@ impl ConnectionSetupRequestError { pub struct BrokenConnectionError(Arc); impl BrokenConnectionError { - /// Retrieve an error reason. - pub fn get_reason(&self) -> &Arc { - &self.0 + /// Retrieve an error reason by downcasting to specific type. + pub fn downcast_ref(&self) -> Option<&T> { + self.0.downcast_ref() } } /// A reason why connection was broken. /// -/// See [`BrokenConnectionError::get_reason()`]. +/// See [`BrokenConnectionError::downcast_ref()`]. /// You can retrieve the actual type by downcasting `Arc`. #[derive(Error, Debug)] #[non_exhaustive] From 2a23f06132b82c78ee38dea9691c0953ef266f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Uzarski?= Date: Tue, 15 Oct 2024 16:51:30 +0200 Subject: [PATCH 3/3] errors: make Display impls of type-erased errors consistent --- scylla-cql/src/types/deserialize/mod.rs | 10 ++-------- scylla-cql/src/types/serialize/mod.rs | 9 ++------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/scylla-cql/src/types/deserialize/mod.rs b/scylla-cql/src/types/deserialize/mod.rs index acdd50c7a4..066e3d9e1a 100644 --- a/scylla-cql/src/types/deserialize/mod.rs +++ b/scylla-cql/src/types/deserialize/mod.rs @@ -179,7 +179,6 @@ pub use row::DeserializeRow; pub use value::DeserializeValue; use std::error::Error; -use std::fmt::Display; use std::sync::Arc; use thiserror::Error; @@ -202,7 +201,7 @@ use thiserror::Error; /// It won't be returned by the `Session` directly, but it might be nested /// in the [`row::BuiltinTypeCheckError`]. #[derive(Debug, Clone, Error)] -#[error(transparent)] +#[error("TypeCheckError: {0}")] pub struct TypeCheckError(pub(crate) Arc); impl TypeCheckError { @@ -233,6 +232,7 @@ impl TypeCheckError { /// It won't be returned by the `Session` directly, but it might be nested /// in the [`row::BuiltinDeserializationError`]. #[derive(Debug, Clone, Error)] +#[error("DeserializationError: {0}")] pub struct DeserializationError(Arc); impl DeserializationError { @@ -248,12 +248,6 @@ impl DeserializationError { } } -impl Display for DeserializationError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "DeserializationError: {}", self.0) - } -} - // This is a hack to enable setting the proper Rust type name in error messages, // even though the error originates from some helper type used underneath. // ASSUMPTION: This should be used: diff --git a/scylla-cql/src/types/serialize/mod.rs b/scylla-cql/src/types/serialize/mod.rs index 27c3c627b5..3bc3424dc0 100644 --- a/scylla-cql/src/types/serialize/mod.rs +++ b/scylla-cql/src/types/serialize/mod.rs @@ -2,7 +2,7 @@ //! Types and traits related to serialization of values to the CQL format. -use std::{error::Error, fmt::Display, sync::Arc}; +use std::{error::Error, sync::Arc}; use thiserror::Error; @@ -33,6 +33,7 @@ pub use writers::{CellValueBuilder, CellWriter, RowWriter}; /// as an argument to the statement, and rewriting it using the new /// `SerializeRow` interface fails. #[derive(Debug, Clone, Error)] +#[error("SerializationError: {0}")] pub struct SerializationError(Arc); impl SerializationError { @@ -47,9 +48,3 @@ impl SerializationError { self.0.downcast_ref() } } - -impl Display for SerializationError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "SerializationError: {}", self.0) - } -}