Skip to content

Commit

Permalink
Move from failure to thiserror
Browse files Browse the repository at this point in the history
Closes flavray#115

This is still a WIP branch, with lots of TODOs and some things about
thiserror I still can't wrap my head around. However, the heavy-lifting
is done, the failure crate has been removed from the list of
dependencies and compilation, tests, benchmarks and linting are all
green.

The two biggest things I have yet to figure out are:
1. How to deal with the errors manually defined in ser.rs and de.rs:
   they are publicly available and as soon as I touch anything I hit
   cryptic serde errors
2. How to make errors like TryFromIntError part of more abstract ones
   like ParseSchemaError, which could have a source error or just a
   String description.
  • Loading branch information
poros committed Jul 3, 2020
1 parent df61e8a commit 33348da
Show file tree
Hide file tree
Showing 14 changed files with 414 additions and 382 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ harness = false
byteorder = "1.0.0"
crc = { version = "1.3.0", optional = true }
digest = "0.9"
failure = "0.1.5"
libflate = "0.1"
num-bigint = "0.2.6"
rand = "0.4"
Expand All @@ -44,6 +43,7 @@ serde = { version = "1.0", features = ["derive"] }
snap = { version = "0.2.3", optional = true }
strum = "0.18.0"
strum_macros = "0.18.0"
thiserror = "1.0"
typed-builder = "0.5.1"
uuid = { version = "0.8.1", features = ["v4"] }
zerocopy = "0.3.0"
Expand Down
5 changes: 2 additions & 3 deletions examples/to_value.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use avro_rs::to_value;
use failure::Error;
use avro_rs::{to_value, AvroError};
use serde::{Deserialize, Serialize};

#[derive(Debug, Deserialize, Serialize)]
Expand All @@ -8,7 +7,7 @@ struct Test {
b: String,
}

fn main() -> Result<(), Error> {
fn main() -> Result<(), AvroError> {
let test = Test {
a: 27,
b: "foo".to_owned(),
Expand Down
14 changes: 7 additions & 7 deletions src/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
use std::io::{Read, Write};
use std::str::FromStr;

use failure::Error;
use libflate::deflate::{Decoder, Encoder};

use crate::errors::AvroError;
use crate::types::{ToAvro, Value};
use crate::util::DecodeError;

/// The compression codec used to compress blocks.
#[derive(Clone, Copy, Debug, PartialEq)]
Expand Down Expand Up @@ -40,27 +39,28 @@ impl ToAvro for Codec {
}

impl FromStr for Codec {
type Err = DecodeError;
type Err = AvroError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"null" => Ok(Codec::Null),
"deflate" => Ok(Codec::Deflate),
#[cfg(feature = "snappy")]
"snappy" => Ok(Codec::Snappy),
_ => Err(DecodeError::new("unrecognized codec")),
_ => Err(AvroError::DecodeError("unrecognized codec".to_string())),
}
}
}

impl Codec {
/// Compress a stream of bytes in-place.
pub fn compress(self, stream: &mut Vec<u8>) -> Result<(), Error> {
pub fn compress(self, stream: &mut Vec<u8>) -> Result<(), AvroError> {
match self {
Codec::Null => (),
Codec::Deflate => {
let mut encoder = Encoder::new(Vec::new());
encoder.write_all(stream)?;
// Deflate errors seem to just be io::Error
*stream = encoder.finish().into_result()?;
}
#[cfg(feature = "snappy")]
Expand All @@ -83,7 +83,7 @@ impl Codec {
}

/// Decompress a stream of bytes in-place.
pub fn decompress(self, stream: &mut Vec<u8>) -> Result<(), Error> {
pub fn decompress(self, stream: &mut Vec<u8>) -> Result<(), AvroError> {
*stream = match self {
Codec::Null => return Ok(()),
Codec::Deflate => {
Expand All @@ -104,7 +104,7 @@ impl Codec {
let actual_crc = crc::crc32::checksum_ieee(&decoded);

if expected_crc != actual_crc {
return Err(DecodeError::new(format!(
return Err(AvroError::DecodeError(format!(
"bad Snappy CRC32; expected {:x} but got {:x}",
expected_crc, actual_crc
))
Expand Down
25 changes: 10 additions & 15 deletions src/decimal.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use failure::{Error, Fail};
use crate::errors::AvroError;
use num_bigint::BigInt;

#[derive(Debug, Clone)]
Expand All @@ -15,38 +15,33 @@ impl PartialEq for Decimal {
}
}

#[derive(Fail, Debug)]
#[fail(display = "Decimal sign extension error: {}", _0)]
pub struct SignExtendError(&'static str, usize, usize);

impl Decimal {
pub(crate) fn len(&self) -> usize {
self.len
}

fn to_vec(&self) -> Result<Vec<u8>, Error> {
fn to_vec(&self) -> Result<Vec<u8>, AvroError> {
self.to_sign_extended_bytes_with_len(self.len)
}

pub(crate) fn to_sign_extended_bytes_with_len(&self, len: usize) -> Result<Vec<u8>, Error> {
pub(crate) fn to_sign_extended_bytes_with_len(&self, len: usize) -> Result<Vec<u8>, AvroError> {
let sign_byte = 0xFF * u8::from(self.value.sign() == num_bigint::Sign::Minus);
let mut decimal_bytes = vec![sign_byte; len];
let raw_bytes = self.value.to_signed_bytes_be();
let num_raw_bytes = raw_bytes.len();
let start_byte_index = len.checked_sub(num_raw_bytes).ok_or_else(|| {
SignExtendError(
"number of bytes requested for sign extension {} is less than the number of bytes needed to decode {}",
len,
num_raw_bytes,
)
})?;
let start_byte_index =
len.checked_sub(num_raw_bytes)
.ok_or_else(|| AvroError::SignExtendError {
requested: len,
needed: num_raw_bytes,
})?;
decimal_bytes[start_byte_index..].copy_from_slice(&raw_bytes);
Ok(decimal_bytes)
}
}

impl std::convert::TryFrom<&Decimal> for Vec<u8> {
type Error = Error;
type Error = AvroError;

fn try_from(decimal: &Decimal) -> Result<Self, Self::Error> {
decimal.to_vec()
Expand Down
54 changes: 30 additions & 24 deletions src/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,32 @@ use std::collections::HashMap;
use std::io::Read;
use std::str::FromStr;

use failure::Error;
use uuid::Uuid;

use crate::decimal::Decimal;
use crate::duration::Duration;
use crate::errors::AvroError;
use crate::schema::Schema;
use crate::types::Value;
use crate::util::{safe_len, zag_i32, zag_i64, DecodeError};
use crate::util::{safe_len, zag_i32, zag_i64};

#[inline]
fn decode_long<R: Read>(reader: &mut R) -> Result<Value, Error> {
fn decode_long<R: Read>(reader: &mut R) -> Result<Value, AvroError> {
zag_i64(reader).map(Value::Long)
}

#[inline]
fn decode_int<R: Read>(reader: &mut R) -> Result<Value, Error> {
fn decode_int<R: Read>(reader: &mut R) -> Result<Value, AvroError> {
zag_i32(reader).map(Value::Int)
}

#[inline]
fn decode_len<R: Read>(reader: &mut R) -> Result<usize, Error> {
fn decode_len<R: Read>(reader: &mut R) -> Result<usize, AvroError> {
zag_i64(reader).and_then(|len| safe_len(len as usize))
}

/// Decode a `Value` from avro format given its `Schema`.
pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, Error> {
pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, AvroError> {
match *schema {
Schema::Null => Ok(Value::Null),
Schema::Boolean => {
Expand All @@ -37,32 +37,34 @@ pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, Error>
match buf[0] {
0u8 => Ok(Value::Boolean(false)),
1u8 => Ok(Value::Boolean(true)),
_ => Err(DecodeError::new("not a bool").into()),
_ => Err(AvroError::DecodeError("not a bool".to_string())),
}
}
Schema::Decimal { ref inner, .. } => match **inner {
Schema::Fixed { .. } => match decode(inner, reader)? {
Value::Fixed(_, bytes) => Ok(Value::Decimal(Decimal::from(bytes))),
_ => Err(DecodeError::new(
"not a fixed value, required for decimal with fixed schema",
)
.into()),
_ => Err(AvroError::DecodeError(
"not a fixed value, required for decimal with fixed schema".to_string(),
)),
},
Schema::Bytes => match decode(inner, reader)? {
Value::Bytes(bytes) => Ok(Value::Decimal(Decimal::from(bytes))),
_ => Err(DecodeError::new(
"not a bytes value, required for decimal with bytes schema",
)
.into()),
_ => Err(AvroError::DecodeError(
"not a bytes value, required for decimal with bytes schema".to_string(),
)),
},
_ => Err(
DecodeError::new("not a fixed or bytes type, required for decimal schema").into(),
),
_ => Err(AvroError::DecodeError(
"not a fixed or bytes type, required for decimal schema".to_string(),
)),
},
Schema::Uuid => Ok(Value::Uuid(Uuid::from_str(
match decode(&Schema::String, reader)? {
Value::String(ref s) => s,
_ => return Err(DecodeError::new("not a string type, required for uuid").into()),
_ => {
return Err(AvroError::DecodeError(
"not a string type, required for uuid".to_string(),
))
}
},
)?)),
Schema::Int => decode_int(reader),
Expand Down Expand Up @@ -100,7 +102,7 @@ pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, Error>

String::from_utf8(buf)
.map(Value::String)
.map_err(|_| DecodeError::new("not a valid utf-8 string").into())
.map_err(|_| AvroError::DecodeError("not a valid utf-8 string".to_string()))
}
Schema::Fixed { size, .. } => {
let mut buf = vec![0u8; size as usize];
Expand Down Expand Up @@ -153,7 +155,9 @@ pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, Error>
let value = decode(inner, reader)?;
items.insert(key, value);
} else {
return Err(DecodeError::new("map key is not a string").into());
return Err(AvroError::DecodeError(
"map key is not a string".to_string(),
));
}
}
}
Expand All @@ -165,7 +169,7 @@ pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, Error>
let variants = inner.variants();
let variant = variants
.get(index as usize)
.ok_or_else(|| DecodeError::new("Union index out of bounds"))?;
.ok_or_else(|| AvroError::DecodeError("Union index out of bounds".to_string()))?;
let value = decode(variant, reader)?;
Ok(Value::Union(Box::new(value)))
}
Expand All @@ -184,10 +188,12 @@ pub fn decode<R: Read>(schema: &Schema, reader: &mut R) -> Result<Value, Error>
let symbol = symbols[index as usize].clone();
Ok(Value::Enum(index, symbol))
} else {
Err(DecodeError::new("enum symbol index out of bounds").into())
Err(AvroError::DecodeError(
"enum symbol index out of bounds".to_string(),
))
}
} else {
Err(DecodeError::new("enum symbol not found").into())
Err(AvroError::DecodeError("enum symbol not found".to_string()))
}
}
}
Expand Down
73 changes: 73 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
use crate::de;
use crate::ser;
use thiserror::Error;

#[derive(Error, Debug)]
/// Error type returned from the library
pub enum AvroError {
/// Represents all cases of `std::io::Error`
#[error(transparent)]
IOError(#[from] std::io::Error),

/// Describes errors happened while decoding Avro data (except for `std::io::Error`)
#[error("Decoding error: {0}")]
DecodeError(String),

/// Describes errors happened while parsing Avro schemas
#[error("Failed to parse schema: {0}")]
ParseSchemaError(String),

/// Describes errors happened while performing schema resolution on Avro data.
#[error("Schema resolution error: {0}")]
SchemaResolutionError(String),

/// Describes errors happened while validating Avro data.
#[error("Validation error: {0}")]
ValidationError(String),

// TODO: figure out how to move the implementation of this error here
/// Describes errors that could be encountered while serializing data
#[error(transparent)]
SerError(#[from] ser::Error),

// TODO: figure out how to move the implementation of this error here
/// Describes errors that could be encountered while serializing data
#[error(transparent)]
DeError(#[from] de::Error),

// TODO: merge with SerError somehow?
// /// Represents all cases of `serde::ser::Error`
//#[error(transparent)]
//SerdeSerError(#[from] serde::ser::Error),
/// Describes errors happened trying to allocate too many bytes
#[error("Unable to allocate {desired} bytes (maximum allowed: {maximum})")]
MemoryAllocationError { desired: usize, maximum: usize },

/// Represents all cases of `uuid::Error`
#[error(transparent)]
UuidError(#[from] uuid::Error),

/// Describe a specific error happening with decimal representation
#[error("Number of bytes requested for decimal sign extension {requested} is less than the number of bytes needed to decode {needed}")]
SignExtendError { requested: usize, needed: usize },

// TODO: Should this be wrapped by ParseSchemaError somehow?
/// Represents all cases of `std::num::TryFromIntError`
#[error(transparent)]
TryFromIntError(#[from] std::num::TryFromIntError),

// TODO: Should this be wrapped by ParseSchemaError somehow?
/// Represents all cases of `serde_json::Error`
#[error(transparent)]
JSONError(#[from] serde_json::Error),

// TODO: Should this be wrapped by SchemaResolutionError somehow?
/// Represents all cases of `std::string::FromUtf8Error`
#[error(transparent)]
FromUtf8Error(#[from] std::string::FromUtf8Error),

/// Represents errors coming from Snappy encoding and decoding
#[cfg(feature = "snappy")]
#[error(transparent)]
SnappyError(#[from] snap::Error),
}
Loading

0 comments on commit 33348da

Please sign in to comment.