From fa709c65f2e5f6f10efa24c786cf9dd157311df5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Wed, 8 Nov 2023 18:54:17 +0100 Subject: [PATCH 1/2] SerializationError: dyn Error instead of dyn Any Error needs to be dynamic, so that users can use their own types, but at the same time allow it to be stored in concrete type (like `QueryError`). `dyn Error` supports downcasting, so it can be used instead of `Any`. --- scylla-cql/src/types/serialize/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scylla-cql/src/types/serialize/mod.rs b/scylla-cql/src/types/serialize/mod.rs index 511a7104b1..8ff2398717 100644 --- a/scylla-cql/src/types/serialize/mod.rs +++ b/scylla-cql/src/types/serialize/mod.rs @@ -1,4 +1,4 @@ -use std::{any::Any, sync::Arc}; +use std::{error::Error, sync::Arc}; pub mod row; pub mod value; @@ -9,4 +9,4 @@ pub use writers::{ CellWriter, CountingWriter, RowWriter, }; -type SerializationError = Arc; +type SerializationError = Arc; From 7bcc461a028a668d8bb2fca9dec869d7727d634f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Tue, 21 Nov 2023 15:22:50 +0100 Subject: [PATCH 2/2] scylla-cql: make SerializationError a newtype instead of alias Implementations such as `impl From for QueryError` don't seem like a good idea when SerializationError is just an alias for `Arc`. Make SerializationError a newtype to avoid such general implementations. --- scylla-cql/src/types/serialize/mod.rs | 12 ++++++++++-- scylla-cql/src/types/serialize/row.rs | 10 ++++++---- scylla-cql/src/types/serialize/value.rs | 16 ++++++++-------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/scylla-cql/src/types/serialize/mod.rs b/scylla-cql/src/types/serialize/mod.rs index 8ff2398717..8e6c91983c 100644 --- a/scylla-cql/src/types/serialize/mod.rs +++ b/scylla-cql/src/types/serialize/mod.rs @@ -1,4 +1,6 @@ -use std::{error::Error, sync::Arc}; +use std::{error::Error, fmt::Display, sync::Arc}; + +use thiserror::Error; pub mod row; pub mod value; @@ -8,5 +10,11 @@ pub use writers::{ BufBackedCellValueBuilder, BufBackedCellWriter, BufBackedRowWriter, CellValueBuilder, CellWriter, CountingWriter, RowWriter, }; +#[derive(Debug, Clone, Error)] +pub struct SerializationError(Arc); -type SerializationError = Arc; +impl Display for SerializationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "SerializationError: {}", self.0) + } +} diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index fe91585c8c..e58bd6d32e 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -72,7 +72,7 @@ pub fn serialize_legacy_row( writer: &mut impl RowWriter, ) -> Result<(), SerializationError> { let serialized = - ::serialized(r).map_err(|err| Arc::new(err) as SerializationError)?; + ::serialized(r).map_err(|err| SerializationError(Arc::new(err)))?; let mut append_value = |value: RawValue| { let cell_writer = writer.make_cell_writer(); @@ -93,9 +93,11 @@ pub fn serialize_legacy_row( for col in ctx.columns() { let val = values_by_name.get(col.name.as_str()).ok_or_else(|| { - Arc::new(ValueListToSerializeRowAdapterError::NoBindMarkerWithName { - name: col.name.clone(), - }) as SerializationError + SerializationError(Arc::new( + ValueListToSerializeRowAdapterError::NoBindMarkerWithName { + name: col.name.clone(), + }, + )) })?; append_value(*val); } diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index 25d605d13d..fde7067265 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -49,14 +49,14 @@ pub fn serialize_legacy_value( ) -> Result { // It's an inefficient and slightly tricky but correct implementation. let mut buf = Vec::new(); - ::serialize(v, &mut buf).map_err(|err| Arc::new(err) as SerializationError)?; + ::serialize(v, &mut buf).map_err(|err| SerializationError(Arc::new(err)))?; // Analyze the output. // All this dance shows how unsafe our previous interface was... if buf.len() < 4 { - return Err(Arc::new(ValueToSerializeCqlAdapterError::TooShort { - size: buf.len(), - })); + return Err(SerializationError(Arc::new( + ValueToSerializeCqlAdapterError::TooShort { size: buf.len() }, + ))); } let (len_bytes, contents) = buf.split_at(4); @@ -66,19 +66,19 @@ pub fn serialize_legacy_value( -1 => Ok(writer.set_null()), len if len >= 0 => { if contents.len() != len as usize { - Err(Arc::new( + Err(SerializationError(Arc::new( ValueToSerializeCqlAdapterError::DeclaredVsActualSizeMismatch { declared: len as usize, actual: contents.len(), }, - )) + ))) } else { Ok(writer.set_value(contents)) } } - _ => Err(Arc::new( + _ => Err(SerializationError(Arc::new( ValueToSerializeCqlAdapterError::InvalidDeclaredSize { size: len }, - )), + ))), } }