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(types): Add gRPC Richer Error Model support (Help) #1293

Merged
merged 5 commits into from
Feb 23, 2023
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
2 changes: 1 addition & 1 deletion tonic-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub use pb::Status;
mod richer_error;

pub use richer_error::{
BadRequest, DebugInfo, ErrorDetail, ErrorDetails, ErrorInfo, FieldViolation,
BadRequest, DebugInfo, ErrorDetail, ErrorDetails, ErrorInfo, FieldViolation, Help, HelpLink,
PreconditionFailure, PreconditionViolation, QuotaFailure, QuotaViolation, RequestInfo,
ResourceInfo, RetryInfo, StatusExt,
};
Expand Down
123 changes: 113 additions & 10 deletions tonic-types/src/richer_error/error_details/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::{collections::HashMap, time};

use super::std_messages::{
BadRequest, DebugInfo, ErrorInfo, FieldViolation, PreconditionFailure, PreconditionViolation,
QuotaFailure, QuotaViolation, RequestInfo, ResourceInfo, RetryInfo,
BadRequest, DebugInfo, ErrorInfo, FieldViolation, Help, HelpLink, PreconditionFailure,
PreconditionViolation, QuotaFailure, QuotaViolation, RequestInfo, ResourceInfo, RetryInfo,
};

pub(crate) mod vec;
Expand Down Expand Up @@ -37,6 +37,9 @@ pub struct ErrorDetails {

/// This field stores [`ResourceInfo`] data, if any.
pub(crate) resource_info: Option<ResourceInfo>,

/// This field stores [`Help`] data, if any.
pub(crate) help: Option<Help>,
}

impl ErrorDetails {
Expand Down Expand Up @@ -83,7 +86,10 @@ impl ErrorDetails {
///
/// let err_details = ErrorDetails::with_debug_info(err_stack, "error details");
/// ```
pub fn with_debug_info(stack_entries: Vec<String>, detail: impl Into<String>) -> Self {
pub fn with_debug_info(
stack_entries: impl Into<Vec<String>>,
detail: impl Into<String>,
) -> Self {
ErrorDetails {
debug_info: Some(DebugInfo::new(stack_entries, detail)),
..ErrorDetails::new()
Expand All @@ -103,7 +109,7 @@ impl ErrorDetails {
/// QuotaViolation::new("subject 2", "description 2"),
/// ]);
/// ```
pub fn with_quota_failure(violations: Vec<QuotaViolation>) -> Self {
pub fn with_quota_failure(violations: impl Into<Vec<QuotaViolation>>) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

FYI these are breaking changes but should be fine since I merge this PR I will release a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we take this opportunity to remove the cloning operations from the ErrorDetails getters (like ErrorDetails::bad_request)? I could do this pushing a commit to #1295, or include this change in the next PR.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if its a breaking change lets get it in, id like to make a breaking change release for tonic soon.

ErrorDetails {
quota_failure: Some(QuotaFailure::new(violations)),
..ErrorDetails::new()
Expand Down Expand Up @@ -176,7 +182,7 @@ impl ErrorDetails {
/// ),
/// ]);
/// ```
pub fn with_precondition_failure(violations: Vec<PreconditionViolation>) -> Self {
pub fn with_precondition_failure(violations: impl Into<Vec<PreconditionViolation>>) -> Self {
ErrorDetails {
precondition_failure: Some(PreconditionFailure::new(violations)),
..ErrorDetails::new()
Expand Down Expand Up @@ -226,7 +232,7 @@ impl ErrorDetails {
/// FieldViolation::new("field_2", "description 2"),
/// ]);
/// ```
pub fn with_bad_request(field_violations: Vec<FieldViolation>) -> Self {
pub fn with_bad_request(field_violations: impl Into<Vec<FieldViolation>>) -> Self {
ErrorDetails {
bad_request: Some(BadRequest::new(field_violations)),
..ErrorDetails::new()
Expand Down Expand Up @@ -311,6 +317,26 @@ impl ErrorDetails {
}
}

/// Generates an [`ErrorDetails`] struct with [`Help`] details and
/// remaining fields set to `None`.
///
/// # Examples
///
/// ```
/// use tonic_types::{ErrorDetails, HelpLink};
///
/// let err_details = ErrorDetails::with_help(vec![
/// HelpLink::new("description of link a", "resource-a.example.local"),
/// HelpLink::new("description of link b", "resource-b.example.local"),
/// ]);
/// ```
pub fn with_help(links: impl Into<Vec<HelpLink>>) -> Self {
ErrorDetails {
help: Some(Help::new(links)),
..ErrorDetails::new()
}
}

/// Get [`RetryInfo`] details, if any.
pub fn retry_info(&self) -> Option<RetryInfo> {
self.retry_info.clone()
Expand Down Expand Up @@ -351,6 +377,11 @@ impl ErrorDetails {
self.resource_info.clone()
}

/// Get [`Help`] details, if any.
pub fn help(&self) -> Option<Help> {
self.help.clone()
}

/// Set [`RetryInfo`] details. Can be chained with other `.set_` and
/// `.add_` [`ErrorDetails`] methods.
///
Expand Down Expand Up @@ -385,7 +416,7 @@ impl ErrorDetails {
/// ```
pub fn set_debug_info(
&mut self,
stack_entries: Vec<String>,
stack_entries: impl Into<Vec<String>>,
detail: impl Into<String>,
) -> &mut Self {
self.debug_info = Some(DebugInfo::new(stack_entries, detail));
Expand All @@ -407,7 +438,7 @@ impl ErrorDetails {
/// QuotaViolation::new("subject 2", "description 2"),
/// ]);
/// ```
pub fn set_quota_failure(&mut self, violations: Vec<QuotaViolation>) -> &mut Self {
pub fn set_quota_failure(&mut self, violations: impl Into<Vec<QuotaViolation>>) -> &mut Self {
self.quota_failure = Some(QuotaFailure::new(violations));
self
}
Expand Down Expand Up @@ -515,7 +546,7 @@ impl ErrorDetails {
/// ```
pub fn set_precondition_failure(
&mut self,
violations: Vec<PreconditionViolation>,
violations: impl Into<Vec<PreconditionViolation>>,
) -> &mut Self {
self.precondition_failure = Some(PreconditionFailure::new(violations));
self
Expand Down Expand Up @@ -601,7 +632,7 @@ impl ErrorDetails {
/// FieldViolation::new("field_2", "description 2"),
/// ]);
/// ```
pub fn set_bad_request(&mut self, violations: Vec<FieldViolation>) -> &mut Self {
pub fn set_bad_request(&mut self, violations: impl Into<Vec<FieldViolation>>) -> &mut Self {
self.bad_request = Some(BadRequest::new(violations));
self
}
Expand Down Expand Up @@ -706,4 +737,76 @@ impl ErrorDetails {
));
self
}

/// Set [`Help`] details. Can be chained with other `.set_` and `.add_`
/// [`ErrorDetails`] methods.
///
/// # Examples
///
/// ```
/// use tonic_types::{ErrorDetails, HelpLink};
///
/// let mut err_details = ErrorDetails::new();
///
/// err_details.set_help(vec![
/// HelpLink::new("description of link a", "resource-a.example.local"),
/// HelpLink::new("description of link b", "resource-b.example.local"),
/// ]);
/// ```
pub fn set_help(&mut self, links: impl Into<Vec<HelpLink>>) -> &mut Self {
self.help = Some(Help::new(links));
self
}

/// Adds a [`HelpLink`] to [`Help`] details. Sets [`Help`] details if it is
/// not set yet. Can be chained with other `.set_` and `.add_`
/// [`ErrorDetails`] methods.
///
/// # Examples
///
/// ```
/// use tonic_types::ErrorDetails;
///
/// let mut err_details = ErrorDetails::new();
///
/// err_details.add_help_link("description of link", "resource.example.local");
/// ```
pub fn add_help_link(
&mut self,
description: impl Into<String>,
url: impl Into<String>,
) -> &mut Self {
match &mut self.help {
Some(help) => {
help.add_link(description, url);
}
None => {
self.help = Some(Help::with_link(description, url));
}
};
self
}

/// Returns `true` if [`Help`] is set and its `links` vector is not empty,
/// otherwise returns `false`.
///
/// # Examples
///
/// ```
/// use tonic_types::ErrorDetails;
///
/// let mut err_details = ErrorDetails::with_help(vec![]);
///
/// assert_eq!(err_details.has_help_links(), false);
///
/// err_details.add_help_link("description of link", "resource.example.local");
///
/// assert_eq!(err_details.has_help_links(), true);
/// ```
pub fn has_help_links(&self) -> bool {
if let Some(help) = &self.help {
return !help.links.is_empty();
}
false
}
}
13 changes: 11 additions & 2 deletions tonic-types/src/richer_error/error_details/vec.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::super::std_messages::{
BadRequest, DebugInfo, ErrorInfo, PreconditionFailure, QuotaFailure, RequestInfo, ResourceInfo,
RetryInfo,
BadRequest, DebugInfo, ErrorInfo, Help, PreconditionFailure, QuotaFailure, RequestInfo,
ResourceInfo, RetryInfo,
};

/// Wraps the structs corresponding to the standard error messages, allowing
Expand Down Expand Up @@ -31,6 +31,9 @@ pub enum ErrorDetail {

/// Wraps the [`ResourceInfo`] struct.
ResourceInfo(ResourceInfo),

/// Wraps the [`Help`] struct.
Help(Help),
}

impl From<RetryInfo> for ErrorDetail {
Expand Down Expand Up @@ -80,3 +83,9 @@ impl From<ResourceInfo> for ErrorDetail {
ErrorDetail::ResourceInfo(err_detail)
}
}

impl From<Help> for ErrorDetail {
fn from(err_detail: Help) -> Self {
ErrorDetail::Help(err_detail)
}
}
63 changes: 58 additions & 5 deletions tonic-types/src/richer_error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use super::pb;

pub use error_details::{vec::ErrorDetail, ErrorDetails};
pub use std_messages::{
BadRequest, DebugInfo, ErrorInfo, FieldViolation, PreconditionFailure, PreconditionViolation,
QuotaFailure, QuotaViolation, RequestInfo, ResourceInfo, RetryInfo,
BadRequest, DebugInfo, ErrorInfo, FieldViolation, Help, HelpLink, PreconditionFailure,
PreconditionViolation, QuotaFailure, QuotaViolation, RequestInfo, ResourceInfo, RetryInfo,
};

trait IntoAny {
Expand Down Expand Up @@ -424,6 +424,28 @@ pub trait StatusExt: crate::sealed::Sealed {
/// }
/// ```
fn get_details_resource_info(&self) -> Option<ResourceInfo>;

/// Get first [`Help`] details found on `tonic::Status`, if any. If some
/// `prost::DecodeError` occurs, returns `None`.
///
/// # Examples
///
/// ```
/// use tonic::{Status, Response};
/// use tonic_types::StatusExt;
///
/// fn handle_request_result<T>(req_result: Result<Response<T>, Status>) {
/// match req_result {
/// Ok(_) => {},
/// Err(status) => {
/// if let Some(help) = status.get_details_help() {
/// // Handle help details
/// }
/// }
/// };
/// }
/// ```
fn get_details_help(&self) -> Option<Help>;
}

impl crate::sealed::Sealed for tonic::Status {}
Expand Down Expand Up @@ -471,6 +493,10 @@ impl StatusExt for tonic::Status {
conv_details.push(resource_info.into_any());
}

if let Some(help) = details.help {
conv_details.push(help.into_any());
}

let details = gen_details_bytes(code, &message, conv_details);

tonic::Status::with_details_and_metadata(code, message, details, metadata)
Expand Down Expand Up @@ -516,6 +542,9 @@ impl StatusExt for tonic::Status {
ErrorDetail::ResourceInfo(res_info) => {
conv_details.push(res_info.into_any());
}
ErrorDetail::Help(help) => {
conv_details.push(help.into_any());
}
}
}

Expand Down Expand Up @@ -568,6 +597,9 @@ impl StatusExt for tonic::Status {
ResourceInfo::TYPE_URL => {
details.resource_info = Some(ResourceInfo::from_any(any)?);
}
Help::TYPE_URL => {
details.help = Some(Help::from_any(any)?);
}
_ => {}
}
}
Expand Down Expand Up @@ -610,6 +642,9 @@ impl StatusExt for tonic::Status {
ResourceInfo::TYPE_URL => {
details.push(ResourceInfo::from_any(any)?.into());
}
Help::TYPE_URL => {
details.push(Help::from_any(any)?.into());
}
_ => {}
}
}
Expand Down Expand Up @@ -732,6 +767,20 @@ impl StatusExt for tonic::Status {

None
}

fn get_details_help(&self) -> Option<Help> {
let status = pb::Status::decode(self.details()).ok()?;

for any in status.details.into_iter() {
if any.type_url.as_str() == Help::TYPE_URL {
if let Ok(detail) = Help::from_any(any) {
return Some(detail);
}
}
}

None
}
}

#[cfg(test)]
Expand All @@ -740,8 +789,8 @@ mod tests {
use tonic::{Code, Status};

use super::{
BadRequest, DebugInfo, ErrorDetails, ErrorInfo, PreconditionFailure, QuotaFailure,
RequestInfo, RetryInfo, StatusExt,
BadRequest, DebugInfo, ErrorDetails, ErrorInfo, Help, PreconditionFailure, QuotaFailure,
RequestInfo, ResourceInfo, RetryInfo, StatusExt,
};

#[test]
Expand All @@ -761,7 +810,9 @@ mod tests {
.set_error_info("SOME_INFO", "example.local", metadata.clone())
.add_precondition_failure_violation("TOS", "example.local", "description")
.add_bad_request_violation("field", "description")
.set_request_info("request-id", "some-request-data");
.set_request_info("request-id", "some-request-data")
.set_resource_info("resource-type", "resource-name", "owner", "description")
.add_help_link("link to resource", "resource.example.local");

let fmt_details = format!("{:?}", err_details);

Expand All @@ -777,6 +828,8 @@ mod tests {
PreconditionFailure::with_violation("TOS", "example.local", "description").into(),
BadRequest::with_violation("field", "description").into(),
RequestInfo::new("request-id", "some-request-data").into(),
ResourceInfo::new("resource-type", "resource-name", "owner", "description").into(),
Help::with_link("link to resource", "resource.example.local").into(),
];

let fmt_details_vec = format!("{:?}", err_details_vec);
Expand Down
6 changes: 4 additions & 2 deletions tonic-types/src/richer_error/std_messages/bad_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ impl BadRequest {
pub const TYPE_URL: &'static str = "type.googleapis.com/google.rpc.BadRequest";

/// Creates a new [`BadRequest`] struct.
pub fn new(field_violations: Vec<FieldViolation>) -> Self {
BadRequest { field_violations }
pub fn new(field_violations: impl Into<Vec<FieldViolation>>) -> Self {
BadRequest {
field_violations: field_violations.into(),
}
}

/// Creates a new [`BadRequest`] struct with a single [`FieldViolation`] in
Expand Down
Loading