Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: Low level (de)ser errors #1017

Merged
merged 5 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions scylla-cql/src/frame/frame_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub enum FrameError {

#[derive(Error, Debug)]
pub enum ParseError {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remind me, is the current plan to get rid of ParseError completely?
If not, we need to remember to add non_exhaustive to it, and other errors (like FrameError above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ultimately would like to get rid of ParseError. It will be decoupled into multiple error types, which will be more specific and what's the most important - they will be typed. As of now, most of the ParseErrors are stringified (e.g. converted to QueryError::InvalidMessage).

There is a similar issue with a FrameError, however I'm not planning to remove it. I'd like to decouple it and introduce a distinction between FrameSerializationError (for client frames) and FrameDeserializationError (for server frames). These two could be then represented as variants of {Query/NewSession}Error (and not stringified to QueryError::InvalidMessage, which is something we currently do).

#[error("Low-level deserialization failed: {0}")]
LowLevelDeserializationError(#[from] LowLevelDeserializationError),
#[error("Could not serialize frame: {0}")]
BadDataToSerialize(String),
#[error("Could not deserialize frame: {0}")]
Expand All @@ -52,3 +54,34 @@ pub enum ParseError {
#[error(transparent)]
CqlTypeError(#[from] CqlTypeError),
}

/// A low level deserialization error.
///
/// This type of error is returned when deserialization
/// of some primitive value fails.
///
/// Possible error kinds:
/// - generic io error - reading from buffer failed
/// - out of range integer conversion
/// - conversion errors - e.g. slice-to-array or primitive-to-enum
/// - not enough bytes in the buffer to deserialize a value
#[non_exhaustive]
#[derive(Error, Debug)]
pub enum LowLevelDeserializationError {
muzarski marked this conversation as resolved.
Show resolved Hide resolved
#[error(transparent)]
IoError(#[from] std::io::Error),
#[error(transparent)]
TryFromIntError(#[from] std::num::TryFromIntError),
#[error("Failed to convert slice into array: {0}")]
TryFromSliceError(#[from] std::array::TryFromSliceError),
#[error("Not enough bytes! expected: {expected}, received: {received}")]
TooFewBytesReceived { expected: usize, received: usize },
#[error("Invalid value length: {0}")]
InvalidValueLength(i32),
#[error("Unknown consistency: {0}")]
UnknownConsistency(#[from] TryFromPrimitiveError<u16>),
#[error("Invalid inet bytes length: {0}")]
InvalidInetLength(u8),
#[error("UTF8 deserialization failed: {0}")]
UTF8DeserializationError(#[from] std::str::Utf8Error),
}
6 changes: 3 additions & 3 deletions scylla-cql/src/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ pub fn parse_response_body_extensions(

let trace_id = if flags & FLAG_TRACING != 0 {
let buf = &mut &*body;
let trace_id = types::read_uuid(buf)?;
let trace_id = types::read_uuid(buf).map_err(frame_errors::ParseError::from)?;
body.advance(16);
Some(trace_id)
} else {
Expand All @@ -198,7 +198,7 @@ pub fn parse_response_body_extensions(
let warnings = if flags & FLAG_WARNING != 0 {
let body_len = body.len();
let buf = &mut &*body;
let warnings = types::read_string_list(buf)?;
let warnings = types::read_string_list(buf).map_err(frame_errors::ParseError::from)?;
let buf_len = buf.len();
body.advance(body_len - buf_len);
warnings
Expand All @@ -209,7 +209,7 @@ pub fn parse_response_body_extensions(
let custom_payload = if flags & FLAG_CUSTOM_PAYLOAD != 0 {
let body_len = body.len();
let buf = &mut &*body;
let payload_map = types::read_bytes_map(buf)?;
let payload_map = types::read_bytes_map(buf).map_err(frame_errors::ParseError::from)?;
let buf_len = buf.len();
body.advance(body_len - buf_len);
Some(payload_map)
Expand Down
2 changes: 1 addition & 1 deletion scylla-cql/src/frame/request/auth_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ impl SerializableRequest for AuthResponse {
const OPCODE: RequestOpcode = RequestOpcode::AuthResponse;

fn serialize(&self, buf: &mut Vec<u8>) -> Result<(), ParseError> {
write_bytes_opt(self.response.as_ref(), buf)
Ok(write_bytes_opt(self.response.as_ref(), buf)?)
}
}
32 changes: 19 additions & 13 deletions scylla-cql/src/frame/response/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::frame::value::{
};
use crate::frame::{frame_errors::ParseError, types};
use crate::types::deserialize::result::{RowIterator, TypedRowIterator};
use crate::types::deserialize::value::{DeserializeValue, MapIterator, UdtIterator};
use crate::types::deserialize::value::{
mk_deser_err, BuiltinDeserializationErrorKind, DeserializeValue, MapIterator, UdtIterator,
};
use crate::types::deserialize::{DeserializationError, FrameSlice};
use bytes::{Buf, Bytes};
use std::borrow::Cow;
Expand Down Expand Up @@ -643,7 +645,10 @@ fn deser_prepared_metadata(buf: &mut &[u8]) -> StdResult<PreparedMetadata, Parse
})
}

pub fn deser_cql_value(typ: &ColumnType, buf: &mut &[u8]) -> StdResult<CqlValue, ParseError> {
pub fn deser_cql_value(
typ: &ColumnType,
buf: &mut &[u8],
) -> StdResult<CqlValue, DeserializationError> {
use ColumnType::*;

if buf.is_empty() {
Expand All @@ -662,10 +667,10 @@ pub fn deser_cql_value(typ: &ColumnType, buf: &mut &[u8]) -> StdResult<CqlValue,

Ok(match typ {
Custom(type_str) => {
return Err(ParseError::BadIncomingData(format!(
"Support for custom types is not yet implemented: {}",
type_str
)));
return Err(mk_deser_err::<CqlValue>(
typ,
BuiltinDeserializationErrorKind::CustomTypeNotSupported(type_str.to_string()),
))
}
Ascii => {
let s = String::deserialize(typ, v)?;
Expand Down Expand Up @@ -784,14 +789,15 @@ pub fn deser_cql_value(typ: &ColumnType, buf: &mut &[u8]) -> StdResult<CqlValue,
Tuple(type_names) => {
let t = type_names
.iter()
.map(|typ| {
types::read_bytes_opt(buf).and_then(|v| {
v.map(|v| {
CqlValue::deserialize(typ, Some(FrameSlice::new_borrowed(v)))
.map_err(Into::into)
})
.map(|typ| -> StdResult<_, DeserializationError> {
let raw = types::read_bytes_opt(buf).map_err(|e| {
mk_deser_err::<CqlValue>(
typ,
BuiltinDeserializationErrorKind::RawCqlBytesReadError(e),
)
})?;
raw.map(|v| CqlValue::deserialize(typ, Some(FrameSlice::new_borrowed(v))))
.transpose()
})
})
.collect::<StdResult<_, _>>()?;
CqlValue::Tuple(t)
Expand Down
Loading
Loading