Skip to content

Commit

Permalink
feat: refactor error to be more clear and maintainable (#226)
Browse files Browse the repository at this point in the history 
* feat: refactor error

* refactor to use ThriftException only

* add from_i32 for application exception kind

* run fmt

* run fmt

* fix comments

* fix comments

* remove commented code
  • Loading branch information
@PureWhiteWu
PureWhiteWu authored Feb 19, 2024
1 parent 8c497c6 commit 9bc9ae3
Show file tree
Hide file tree
Showing 39 changed files with 2,720 additions and 2,894 deletions.
183 changes: 91 additions & 92 deletions Cargo.lock
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pilota-build/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pilota-build"
version = "0.10.2"
version = "0.11.0"
edition = "2021"
description = "Compile thrift and protobuf idl into rust code at compile-time."
documentation = "https://docs.rs/pilota-build"
Expand All @@ -17,7 +17,7 @@ keywords = ["serialization", "thrift", "protobuf", "volo"]
maintenance = { status = "actively-developed" }

[dependencies]
pilota-thrift-parser = { path = "../pilota-thrift-parser", version = "0.10" }
pilota-thrift-parser = { path = "../pilota-thrift-parser", version = "0.11" }

ahash = "0.8"
anyhow = "1"
Expand Down
49 changes: 22 additions & 27 deletions pilota-build/src/codegen/thrift/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,23 @@ impl ThriftBackend {
// r#"fn decode_async<'a, T: ::pilota::thrift::TAsyncInputProtocol>(
// protocol: &'a mut T,
// ) -> ::std::pin::Pin<::std::boxed::Box<dyn ::std::future::Future<Output =
// ::std::result::Result<Self, ::pilota::thrift::DecodeError>> + Send +
// 'a>> {{ ::std::boxed::Box::pin(async move {{ {decode_async} }})
// }}"#
// ::std::result::Result<Self, ::pilota::thrift::ThriftException>> + Send
// + 'a>> {{ ::std::boxed::Box::pin(async move {{ {decode_async}
// }}) }}"#
// )
// } else {
// format!(
// r#"async fn decode_async<T: ::pilota::thrift::TAsyncInputProtocol>(
// protocol: &mut T,
// ) -> ::std::result::Result<Self,::pilota::thrift::DecodeError> {{
// ) -> ::std::result::Result<Self,::pilota::thrift::ThriftException> {{
// {decode_async}
// }}"#
// )
// };
let decode_async_fn = format!(
r#"fn decode_async<'a, T: ::pilota::thrift::TAsyncInputProtocol>(
protocol: &'a mut T,
) -> ::std::pin::Pin<::std::boxed::Box<dyn ::std::future::Future<Output = ::std::result::Result<Self, ::pilota::thrift::DecodeError>> + Send + 'a>> {{
) -> ::std::pin::Pin<::std::boxed::Box<dyn ::std::future::Future<Output = ::std::result::Result<Self, ::pilota::thrift::ThriftException>> + Send + 'a>> {{
::std::boxed::Box::pin(async move {{
{decode_async}
}})
Expand All @@ -135,15 +135,15 @@ impl ThriftBackend {
fn encode<T: ::pilota::thrift::TOutputProtocol>(
&self,
protocol: &mut T,
) -> ::std::result::Result<(),::pilota::thrift::EncodeError> {{
) -> ::std::result::Result<(),::pilota::thrift::ThriftException> {{
#[allow(unused_imports)]
use ::pilota::thrift::TOutputProtocolExt;
{encode}
}}
fn decode<T: ::pilota::thrift::TInputProtocol>(
protocol: &mut T,
) -> ::std::result::Result<Self,::pilota::thrift::DecodeError> {{
) -> ::std::result::Result<Self,::pilota::thrift::ThriftException> {{
#[allow(unused_imports)]
use ::pilota::{{thrift::TLengthProtocolExt, Buf}};
{decode}
Expand Down Expand Up @@ -278,8 +278,8 @@ impl ThriftBackend {
format!(
r#"let Some({s}) = {s} else {{
return Err(
::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::InvalidData,
::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"field {s} is required".to_string()
)
)
Expand All @@ -292,14 +292,14 @@ impl ThriftBackend {
format! {
r#"async {{
{read_fields}
Ok::<_, ::pilota::thrift::DecodeError>(())
Ok::<_, ::pilota::thrift::ThriftException>(())
}}.await"#
}
} else {
format! {
r#"(|| {{
{read_fields}
Ok::<_, ::pilota::thrift::DecodeError>(())
Ok::<_, ::pilota::thrift::ThriftException>(())
}})()"#
}
};
Expand All @@ -314,19 +314,14 @@ impl ThriftBackend {
let mut __pilota_decoding_field_id = None;
{read_struct_begin};
if let Err(err) = {read_fields} {{
if let Err(mut err) = {read_fields} {{
if let Some(field_id) = __pilota_decoding_field_id {{
return Err(::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::WithContext(::std::boxed::Box::new(err)),
format!("{format_msg}", field_id),
));
}} else {{
return Err(err)
err.prepend_msg(&format!("{format_msg}, caused by: ", field_id));
}}
return Err(err);
}};
{read_struct_end};
{verify_required_fields}
{set_default_fields}
Expand Down Expand Up @@ -535,8 +530,8 @@ impl CodegenBackend for ThriftBackend {
format! {
r#"let value = {read_i32};
Ok(::std::convert::TryFrom::try_from(value).map_err(|err|
::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::InvalidData,
::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
format!("{err_msg_tmpl}", value)
))?)"#
}
Expand Down Expand Up @@ -680,8 +675,8 @@ impl CodegenBackend for ThriftBackend {
{decode_len}
ret = Some({name}::{variant_name}(field_ident));
}} else {{
return Err(::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::InvalidData,
return Err(::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"received multiple fields for union from remote Message"
));
}}
Expand All @@ -699,8 +694,8 @@ impl CodegenBackend for ThriftBackend {
ret = Some({name}::_UnknownFields(__pilota_linked_bytes));
}}
}} else {{
return Err(::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::InvalidData,
return Err(::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"received multiple fields for union from remote Message"
));
}}"#
Expand All @@ -713,8 +708,8 @@ impl CodegenBackend for ThriftBackend {
if e.variants.first().filter(|v| variant_is_void(v)).is_some() {
format!("Ok({name}::Ok(()))").into()
} else {
r#"Err(::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::InvalidData,
r#"Err(::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"received empty union from remote Message")
)"#.into()
};
Expand Down
2 changes: 1 addition & 1 deletion pilota-build/src/test/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,6 @@ mod tests {

let err = A::decode(&mut TBinaryProtocol::new(&mut data, false)).unwrap_err();

assert_eq!(format!("{}", err), "decode struct `A` field(#1) failed, caused by decode struct `B` field(#1) failed, caused by invalid ttype 100")
assert_eq!(err.to_string(), "Protocol(ProtocolException { kind: InvalidData, message: \"decode struct `A` field(#1) failed, caused by: decode struct `B` field(#1) failed, caused by: invalid ttype 100\" })")
}
}
36 changes: 15 additions & 21 deletions pilota-build/test_data/must_gen_items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub mod must_gen_items {
fn encode<T: ::pilota::thrift::TOutputProtocol>(
&self,
protocol: &mut T,
) -> ::std::result::Result<(), ::pilota::thrift::EncodeError> {
) -> ::std::result::Result<(), ::pilota::thrift::ThriftException> {
#[allow(unused_imports)]
use ::pilota::thrift::TOutputProtocolExt;
let struct_ident = ::pilota::thrift::TStructIdentifier { name: "A" };
Expand All @@ -26,7 +26,7 @@ pub mod must_gen_items {

fn decode<T: ::pilota::thrift::TInputProtocol>(
protocol: &mut T,
) -> ::std::result::Result<Self, ::pilota::thrift::DecodeError> {
) -> ::std::result::Result<Self, ::pilota::thrift::ThriftException> {
#[allow(unused_imports)]
use ::pilota::{thrift::TLengthProtocolExt, Buf};

Expand All @@ -35,7 +35,7 @@ pub mod must_gen_items {
let mut __pilota_decoding_field_id = None;

protocol.read_struct_begin()?;
if let Err(err) = (|| {
if let Err(mut err) = (|| {
loop {
let field_ident = protocol.read_field_begin()?;
if field_ident.field_type == ::pilota::thrift::TType::Stop {
Expand All @@ -57,18 +57,15 @@ pub mod must_gen_items {
protocol.read_field_end()?;
protocol.field_end_len();
}
Ok::<_, ::pilota::thrift::DecodeError>(())
Ok::<_, ::pilota::thrift::ThriftException>(())
})() {
if let Some(field_id) = __pilota_decoding_field_id {
return Err(::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::WithContext(::std::boxed::Box::new(
err,
)),
format!("decode struct `A` field(#{}) failed", field_id),
err.prepend_msg(&format!(
"decode struct `A` field(#{}) failed, caused by: ",
field_id
));
} else {
return Err(err);
}
return Err(err);
};
protocol.read_struct_end()?;

Expand All @@ -81,7 +78,7 @@ pub mod must_gen_items {
) -> ::std::pin::Pin<
::std::boxed::Box<
dyn ::std::future::Future<
Output = ::std::result::Result<Self, ::pilota::thrift::DecodeError>,
Output = ::std::result::Result<Self, ::pilota::thrift::ThriftException>,
> + Send
+ 'a,
>,
Expand All @@ -92,7 +89,7 @@ pub mod must_gen_items {
let mut __pilota_decoding_field_id = None;

protocol.read_struct_begin().await?;
if let Err(err) = async {
if let Err(mut err) = async {
loop {
let field_ident = protocol.read_field_begin().await?;
if field_ident.field_type == ::pilota::thrift::TType::Stop {
Expand All @@ -113,20 +110,17 @@ pub mod must_gen_items {

protocol.read_field_end().await?;
}
Ok::<_, ::pilota::thrift::DecodeError>(())
Ok::<_, ::pilota::thrift::ThriftException>(())
}
.await
{
if let Some(field_id) = __pilota_decoding_field_id {
return Err(::pilota::thrift::DecodeError::new(
::pilota::thrift::DecodeErrorKind::WithContext(
::std::boxed::Box::new(err),
),
format!("decode struct `A` field(#{}) failed", field_id),
err.prepend_msg(&format!(
"decode struct `A` field(#{}) failed, caused by: ",
field_id
));
} else {
return Err(err);
}
return Err(err);
};
protocol.read_struct_end().await?;

Expand Down
Loading

0 comments on commit 9bc9ae3

Please sign in to comment.