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.

Sign up for GitHub

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

Jump to bottom

feat: Add Status with Details Constructor #308

Merged
merged 3 commits into from
Mar 30, 2020
Merged

feat: Add Status with Details Constructor #308

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
537fa57
Give an option for creating errors with binary Details field
Mar 29, 2020
c347c88
squashme rustfmt
Mar 29, 2020
f6e71d5
Remove prost dependency and only expose with_details constructor that…
Mar 29, 2020
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
19 changes: 19 additions & 0 deletions tonic/src/status.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use bytes::Bytes;
use http::header::{HeaderMap, HeaderValue};
use percent_encoding::{percent_decode, percent_encode, AsciiSet, CONTROLS};
use prost::Message;
use std::{borrow::Cow, error::Error, fmt};
use tracing::{debug, trace, warn};

Expand Down Expand Up @@ -401,6 +402,24 @@ impl Status {

Ok(())
}

/// Create a new `Status` with the associated code, message, and binary details field.
pub fn with_raw_details(code: Code, message: impl Into<String>, details: Bytes) -> Status {
Status {
code,
message: message.into(),
details: details,
}
}

/// Create a new `Status` with the associated code, message, and protobuf details field.
pub fn with_details(code: Code, message: impl Into<String>, details: impl Message) -> Status {
Copy link
Member

Choose a reason for hiding this comment

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

I actually believe we should not include a prost based one here. I think we can just have the one above that takes a Bytes and we remove the impl Into. We can also rename the one above to with_details. then let the user choose if they want to encode via prost or something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Just copy pasta from the PR I inherited. I'm fairly new to Rust so I appreciate your patience. Is this cargo check --no-default-features CI check to enforce that there's no unnecessary coupling of external crates to the core library?

let mut bytes = vec![];
details
.encode(&mut bytes)
.expect("Message only errors if not enough space");
Status::with_raw_details(code, message, bytes.into())
}
}

impl fmt::Debug for Status {
Expand Down