Skip to content

Commit

Permalink
refactor(rust): simplify error handle (#1823)
Browse files Browse the repository at this point in the history
## What does this PR do?

Make error easy to use.

In most case, users don't care about error details, so too many fields
in enum is hard to write, and hard to use.

So I refactor it to include an `Other` field to be used as a general
Error, most error can be mapped to it directly.

`Ref` is a special case, so I leave it as it's now.

## Related issues


## Does this PR introduce any user-facing change?

- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?

## Benchmark
  • Loading branch information
jiacai2050 authored Sep 4, 2024
1 parent 0af2084 commit f8e1db6
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 121 deletions.
1 change: 1 addition & 0 deletions rust/fury-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ quote = { default-features = false, version = "1.0" }
byteorder = { version = "1.4" }
chrono = "0.4"
thiserror = { default-features = false, version = "1.0" }
anyhow = "1"
num_enum = "0.5.1"


Expand Down
85 changes: 28 additions & 57 deletions rust/fury-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,67 +15,38 @@
// specific language governing permissions and limitations
// under the License.

use super::types::Language;
use thiserror::Error;

#[derive(thiserror::Error, Debug)]
#[derive(Error, Debug)]
#[non_exhaustive]
pub enum Error {
#[error("Field is not Option type, can't be deserialize of None")]
Null,

#[error("Fury on Rust not support Ref type")]
Ref,

#[error("Fury on Rust not support RefValue type")]
RefValue,

#[error("BadRefFlag")]
BadRefFlag,

#[error("Bad FieldType; expected: {expected:?}, actual: {actual:?}")]
FieldType { expected: i16, actual: i16 },

#[error("Bad timestamp; out-of-range number of milliseconds")]
NaiveDateTime,

#[error("Bad date; out-of-range")]
NaiveDate,

#[error("Schema is not consistent; expected: {expected:?}, actual: {actual:?}")]
StructHash { expected: u32, actual: u32 },

#[error("Bad Tag Type: {0}")]
TagType(u8),

#[error("Only Xlang supported; receive: {language:?}")]
UnsupportedLanguage { language: Language },

#[error("Unsupported Language Code; receive: {code:?}")]
UnsupportedLanguageCode { code: u8 },

#[error("Unsupported encoding of field name in type meta; receive: {code:?}")]
UnsupportedTypeMetaFieldNameEncoding { code: u8 },

#[error("encoded_data cannot be empty")]
EncodedDataEmpty,

#[error("Long meta string than 32767 is not allowed")]
LengthExceed,

#[error("Non-ASCII characters in meta string are not allowed")]
OnlyAllowASCII,

#[error("Unsupported character for LOWER_SPECIAL encoding: {ch:?}")]
UnsupportedLowerSpecialCharacter { ch: char },

#[error("Unsupported character for LOWER_UPPER_DIGIT_SPECIAL encoding: {ch:?}")]
UnsupportedLowerUpperDigitSpecialCharacter { ch: char },

#[error("Invalid character value for LOWER_SPECIAL decoding: {value:?}")]
InvalidLowerSpecialValue { value: u8 },

#[error("Invalid character value for LOWER_UPPER_DIGIT_SPECIAL decoding: {value:?}")]
InvalidLowerUpperDigitSpecialValue { value: u8 },
#[error(transparent)]
Other(#[from] anyhow::Error),
}

#[error("Unregistered type when serializing or deserializing object of Any type: {value:?}")]
UnregisteredType { value: u32 },
/// Works like anyhow's [ensure](https://docs.rs/anyhow/latest/anyhow/macro.ensure.html)
/// But return `Return<T, ErrorFromAnyhow>`
#[macro_export]
macro_rules! ensure {
($cond:expr, $msg:literal) => {
if !$cond {
return Err(anyhow::anyhow!($msg).into());
}
};
($cond:expr, $err:expr) => {
if !$cond {
return Err($err.into());
}
};
($cond:expr, $fmt:expr, $($arg:tt)*) => {
if !$cond {
return Err(anyhow::anyhow!($fmt, $($arg)*).into());
}
};
}

// Re-export anyhow::Error since it may appear in the public API.
pub use anyhow::Error as AnyhowError;
59 changes: 39 additions & 20 deletions rust/fury-core/src/meta/meta_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,15 @@
// specific language governing permissions and limitations
// under the License.

use anyhow::anyhow;

use crate::ensure;
use crate::error::Error;
use crate::meta::string_util;

// equal to "std::i16::MAX"
const SHORT_MAX_VALUE: usize = 32767;

#[derive(Debug, PartialEq)]
pub enum Encoding {
Utf8 = 0x00,
Expand All @@ -39,9 +45,8 @@ impl MetaString {
pub fn new(original: String, encoding: Encoding, bytes: Vec<u8>) -> Result<Self, Error> {
let mut strip_last_char = false;
if encoding != Encoding::Utf8 {
if bytes.is_empty() {
return Err(Error::EncodedDataEmpty);
}
ensure!(!bytes.is_empty(), anyhow!("Encoded data cannot be empty"));

strip_last_char = (bytes[0] & 0x80) != 0;
}
Ok(MetaString {
Expand Down Expand Up @@ -88,11 +93,13 @@ impl MetaStringEncoder {
if input.is_empty() {
return MetaString::new(input.to_string(), Encoding::Utf8, vec![]);
}
// equal to "std::i16::MAX"
const SHORT_MAX_VALUE: usize = 32767;
if input.len() >= SHORT_MAX_VALUE {
return Err(Error::LengthExceed);
}
ensure!(
input.len() < SHORT_MAX_VALUE,
anyhow!(
"Meta string is too long, max:{SHORT_MAX_VALUE}, current:{}",
input.len()
)
);
if !self.is_latin(input) {
return MetaString::new(input.to_string(), Encoding::Utf8, input.as_bytes().to_vec());
}
Expand Down Expand Up @@ -164,14 +171,18 @@ impl MetaStringEncoder {
if input.is_empty() {
return MetaString::new(input.to_string(), Encoding::Utf8, vec![]);
}
// equal to "std::i16::MAX"
const SHORT_MAX_VALUE: usize = 32767;
if input.len() >= SHORT_MAX_VALUE {
return Err(Error::LengthExceed);
}
if encoding != Encoding::Utf8 && !self.is_latin(input) {
return Err(Error::OnlyAllowASCII);
};
ensure!(
input.len() < SHORT_MAX_VALUE,
anyhow!(
"Meta string is too long, max:{SHORT_MAX_VALUE}, current:{}",
input.len()
)
);
ensure!(
encoding == Encoding::Utf8 || self.is_latin(input),
anyhow!("Non-ASCII characters in meta string are not allowed")
);

if input.is_empty() {
return MetaString::new(input.to_string(), Encoding::Utf8, vec![]);
};
Expand Down Expand Up @@ -261,7 +272,9 @@ impl MetaStringEncoder {
'_' => Ok(27),
'$' => Ok(28),
'|' => Ok(29),
_ => Err(Error::UnsupportedLowerSpecialCharacter { ch: c }),
_ => Err(anyhow!(
"Unsupported character for LOWER_UPPER_DIGIT_SPECIAL encoding: {c}"
))?,
},
6 => match c {
'a'..='z' => Ok(c as u8 - b'a'),
Expand All @@ -273,7 +286,9 @@ impl MetaStringEncoder {
} else if c == '_' {
Ok(63)
} else {
Err(Error::UnsupportedLowerUpperDigitSpecialCharacter { ch: c })
Err(anyhow!(
"Invalid character value for LOWER_SPECIAL decoding: {c:?}"
))?
}
}
},
Expand Down Expand Up @@ -362,7 +377,9 @@ impl MetaStringDecoder {
27 => Ok('_'),
28 => Ok('$'),
29 => Ok('|'),
_ => Err(Error::InvalidLowerSpecialValue { value: char_value }),
_ => Err(anyhow!(
"Invalid character value for LOWER_SPECIAL decoding: {char_value}"
))?,
}
}

Expand All @@ -373,7 +390,9 @@ impl MetaStringDecoder {
52..=61 => Ok((b'0' + char_value - 52) as char),
62 => Ok('.'),
63 => Ok('_'),
_ => Err(Error::InvalidLowerUpperDigitSpecialValue { value: char_value }),
_ => Err(anyhow!(
"Invalid character value for LOWER_UPPER_DIGIT_SPECIAL decoding: {char_value}"
))?,
}
}

Expand Down
5 changes: 4 additions & 1 deletion rust/fury-core/src/meta/type_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use super::meta_string::MetaStringEncoder;
use crate::buffer::{Reader, Writer};
use crate::error::Error;
use crate::meta::{Encoding, MetaStringDecoder};
use anyhow::anyhow;

pub struct FieldInfo {
field_name: String,
Expand All @@ -38,7 +39,9 @@ impl FieldInfo {
0x00 => Ok(Encoding::Utf8),
0x01 => Ok(Encoding::AllToLowerSpecial),
0x02 => Ok(Encoding::LowerUpperDigitSpecial),
_ => Err(Error::UnsupportedTypeMetaFieldNameEncoding { code: value }),
_ => Err(anyhow!(
"Unsupported encoding of field name in type meta, value:{value}"
))?,
}
}

Expand Down
3 changes: 2 additions & 1 deletion rust/fury-core/src/resolver/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use crate::buffer::{Reader, Writer};
use crate::error::Error;
use crate::fury::Fury;
use anyhow::anyhow;

use crate::meta::TypeMeta;
use crate::resolver::meta_resolver::{MetaReaderResolver, MetaWriterResolver};
Expand Down Expand Up @@ -119,7 +120,7 @@ impl<'de, 'bf: 'de> ReadContext<'de, 'bf> {
self.tags.push(tag);
Ok(tag)
} else {
Err(Error::TagType(tag_type))
Err(anyhow!("Unknown tag type, value:{tag_type}"))?
}
}
}
20 changes: 11 additions & 9 deletions rust/fury-core/src/row/row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

use crate::util::EPOCH;
use crate::{buffer::Writer, error::Error};
use anyhow::anyhow;
use byteorder::{ByteOrder, LittleEndian};
use chrono::{DateTime, Days, NaiveDate, NaiveDateTime};
use std::collections::BTreeMap;
Expand Down Expand Up @@ -95,10 +96,11 @@ impl<'a> Row<'a> for NaiveDate {

fn cast(bytes: &[u8]) -> Self::ReadResult {
let days = LittleEndian::read_u32(bytes);
match EPOCH.checked_add_days(Days::new(days.into())) {
Some(value) => Ok(value),
None => Err(Error::NaiveDate),
}
EPOCH
.checked_add_days(Days::new(days.into()))
.ok_or(Error::from(anyhow!(
"Date out of range, {days} days since epoch"
)))
}
}

Expand All @@ -111,11 +113,11 @@ impl<'a> Row<'a> for NaiveDateTime {

fn cast(bytes: &[u8]) -> Self::ReadResult {
let timestamp = LittleEndian::read_u64(bytes);
let ret = DateTime::from_timestamp_millis(timestamp as i64).map(|dt| dt.naive_utc());
match ret {
Some(r) => Ok(r),
None => Err(Error::NaiveDateTime),
}
DateTime::from_timestamp_millis(timestamp as i64)
.map(|dt| dt.naive_utc())
.ok_or(Error::from(anyhow!(
"Date out of range, timestamp:{timestamp}"
)))
}
}

Expand Down
5 changes: 3 additions & 2 deletions rust/fury-core/src/serializer/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::fury::Fury;
use crate::resolver::context::{ReadContext, WriteContext};
use crate::serializer::Serializer;
use crate::types::{FieldType, Mode, RefFlag};
use anyhow::anyhow;
use std::any::Any;

impl Serializer for Box<dyn Any> {
Expand Down Expand Up @@ -75,13 +76,13 @@ impl Serializer for Box<dyn Any> {
.get_deserializer()(context)
}
} else if ref_flag == (RefFlag::Null as i8) {
Err(Error::Null)
Err(anyhow!("Try to deserialize `any` to null"))?
} else if ref_flag == (RefFlag::Ref as i8) {
reset_cursor(&mut context.reader);
Err(Error::Ref)
} else {
reset_cursor(&mut context.reader);
Err(Error::BadRefFlag)
Err(anyhow!("Unknown ref flag, value:{ref_flag}"))?
}
}
}
20 changes: 11 additions & 9 deletions rust/fury-core/src/serializer/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@ use crate::resolver::context::WriteContext;
use crate::serializer::Serializer;
use crate::types::{FieldType, FuryGeneralList};
use crate::util::EPOCH;
use anyhow::anyhow;
use chrono::{DateTime, Days, NaiveDate, NaiveDateTime};
use std::mem;

impl Serializer for NaiveDateTime {
fn read(context: &mut ReadContext) -> Result<Self, Error> {
let timestamp = context.reader.u64();
let ret = DateTime::from_timestamp_millis(timestamp as i64).map(|dt| dt.naive_utc());
match ret {
Some(r) => Ok(r),
None => Err(Error::NaiveDateTime),
}
DateTime::from_timestamp_millis(timestamp as i64)
.map(|dt| dt.naive_utc())
.ok_or(Error::from(anyhow!(
"Date out of range, timestamp:{timestamp}"
)))
}

fn write(&self, context: &mut WriteContext) {
Expand Down Expand Up @@ -62,10 +63,11 @@ impl Serializer for NaiveDate {

fn read(context: &mut ReadContext) -> Result<Self, Error> {
let days = context.reader.u64();
match EPOCH.checked_add_days(Days::new(days)) {
Some(value) => Ok(value),
None => Err(Error::NaiveDate),
}
EPOCH
.checked_add_days(Days::new(days))
.ok_or(Error::from(anyhow!(
"Date out of range, {days} days since epoch"
)))
}

fn get_type_id(_fury: &Fury) -> i16 {
Expand Down
Loading

0 comments on commit f8e1db6

Please sign in to comment.