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

feat: refactor error to be more clear and maintainable #226

Merged
merged 8 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
183 changes: 91 additions & 92 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pilota-build/Cargo.toml
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
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
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
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
Loading