You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by al...@apache.org on 2021/03/06 22:39:13 UTC
[thrift] branch master updated: THRIFT-5360 Remove deprecated
Error::description() methods (#2342)
This is an automated email from the ASF dual-hosted git repository.
allengeorge pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new a194276 THRIFT-5360 Remove deprecated Error::description() methods (#2342)
a194276 is described below
commit a194276fab1bbdbf6e9e45bbfd2db0b4cd327648
Author: Allen George <al...@apache.org>
AuthorDate: Sat Mar 6 17:39:02 2021 -0500
THRIFT-5360 Remove deprecated Error::description() methods (#2342)
Client: rs
---
compiler/cpp/src/thrift/generate/t_rs_generator.cc | 21 +++---
lib/rs/README.md | 84 ++++++++++++++++++++++
lib/rs/src/errors.rs | 77 +++++++-------------
3 files changed, 120 insertions(+), 62 deletions(-)
diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
index f14dd6f..78064fd 100644
--- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
@@ -1087,15 +1087,7 @@ void t_rs_generator::render_struct_definition(
void t_rs_generator::render_exception_struct_error_trait_impls(const string& struct_name, t_struct* tstruct) {
// error::Error trait
- f_gen_ << "impl Error for " << struct_name << " {" << endl;
- indent_up();
- f_gen_ << indent() << "fn description(&self) -> &str {" << endl;
- indent_up();
- f_gen_ << indent() << "\"" << "remote service threw " << tstruct->get_name() << "\"" << endl; // use *original* name
- indent_down();
- f_gen_ << indent() << "}" << endl;
- indent_down();
- f_gen_ << "}" << endl;
+ f_gen_ << "impl Error for " << struct_name << " {}" << endl;
f_gen_ << endl;
// convert::From trait
@@ -1115,7 +1107,12 @@ void t_rs_generator::render_exception_struct_error_trait_impls(const string& str
indent_up();
f_gen_ << indent() << "fn fmt(&self, f: &mut Formatter) -> fmt::Result {" << endl;
indent_up();
- f_gen_ << indent() << "self.description().fmt(f)" << endl;
+ f_gen_
+ << indent()
+ << "write!(f, "
+ << "\"remote service threw " << tstruct->get_name() << "\"" // use *original* name
+ << ")"
+ << endl;
indent_down();
f_gen_ << indent() << "}" << endl;
indent_down();
@@ -2896,7 +2893,7 @@ void t_rs_generator::render_sync_handler_failed_user_exception_branch(t_function
f_gen_ << indent() << "let ret_err = {" << endl;
indent_up();
- render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "usr_err.description()");
+ render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "usr_err.to_string()");
indent_down();
f_gen_ << indent() << "};" << endl;
render_sync_handler_send_exception_response(tfunc, "ret_err");
@@ -2919,7 +2916,7 @@ void t_rs_generator::render_sync_handler_failed_application_exception_branch(
void t_rs_generator::render_sync_handler_failed_default_exception_branch(t_function *tfunc) {
f_gen_ << indent() << "let ret_err = {" << endl;
indent_up();
- render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "e.description()");
+ render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "e.to_string()");
indent_down();
f_gen_ << indent() << "};" << endl;
if (tfunc->is_oneway()) {
diff --git a/lib/rs/README.md b/lib/rs/README.md
index 1e10e35..30e36d4 100644
--- a/lib/rs/README.md
+++ b/lib/rs/README.md
@@ -48,6 +48,90 @@ Breaking changes are minimized. When they are made they will be outlined below w
##### Thrift 0.15.0
+* **[THRIFT-5360]** - No longer define OR generate `description()` methods for `Error` types.
+
+ `Error.description()` was soft-deprecated in 1.27, and deprecated as of 1.41.
+ Library error types also do not implement `Error.description()`. Also, as a result of this change
+ the generated Rust representation of an Error no longer implements the `Error.description()` method.
+ Instead, it generates a `Display` impl with the same information.
+
+ For example:
+
+ ```thrift
+ exception Xception {
+ 1: i32 errorCode,
+ 2: string message
+ }
+ ```
+
+ used to generate:
+
+ ```rust
+ use std::error::Error;
+ use std::fmt;
+ use std::fmt::{Display, Formatter};
+
+ // auto-generated by the Thrift compiler
+ #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+ pub struct Xception {
+ pub error_code: Option<i32>,
+ pub message: Option<String>,
+ }
+
+ // auto-generated by the Thrift compiler
+ impl Error for Xception {
+ fn description(&self) -> &str {
+ "remote service threw Xception"
+ }
+ }
+
+ // auto-generated by the Thrift compiler
+ impl From<Xception> for thrift::Error {
+ fn from(e: Xception) -> Self {
+ thrift::Error::User(Box::new(e))
+ }
+ }
+
+ // auto-generated by the Thrift compiler
+ impl Display for Xception {
+ fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+ self.description().format(f)
+ }
+ }
+ ```
+
+ It *now* generates:
+
+ ```rust
+ use std::error::Error;
+ use std::fmt;
+ use std::fmt::{Display, Formatter};
+
+ // auto-generated by the Thrift compiler
+ #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+ pub struct Xception {
+ pub error_code: Option<i32>,
+ pub message: Option<String>,
+ }
+
+ // auto-generated by the Thrift compiler
+ impl Error for Xception { }
+
+ // auto-generated by the Thrift compiler
+ impl From<Xception> for thrift::Error {
+ fn from(e: Xception) -> Self {
+ thrift::Error::User(Box::new(e))
+ }
+ }
+
+ // auto-generated by the Thrift compiler
+ impl Display for Xception {
+ fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+ write!(f, "remote service threw Xception")
+ }
+ }
+ ```
+
* **[THRIFT-5314]** - Generate enums implementations that are forward compatible (i.e. don't error on unknown values)
As a result of this change the Rust representation of an enum changes from a standard
diff --git a/lib/rs/src/errors.rs b/lib/rs/src/errors.rs
index 03d98cf..fc26330 100644
--- a/lib/rs/src/errors.rs
+++ b/lib/rs/src/errors.rs
@@ -17,7 +17,6 @@
use std::convert::TryFrom;
use std::convert::{From, Into};
-use std::error::Error as StdError;
use std::fmt::{Debug, Display, Formatter};
use std::{error, fmt, io, string};
@@ -146,11 +145,7 @@ use crate::protocol::{
/// }
///
/// // auto-generated by the Thrift compiler
-/// impl Error for Xception {
-/// fn description(&self) -> &str {
-/// "remote service threw Xception"
-/// }
-/// }
+/// impl Error for Xception { }
///
/// // auto-generated by the Thrift compiler
/// impl From<Xception> for thrift::Error {
@@ -162,7 +157,7 @@ use crate::protocol::{
/// // auto-generated by the Thrift compiler
/// impl Display for Xception {
/// fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-/// self.description().fmt(f)
+/// write!(f, "remote service threw Xception")
/// }
/// }
///
@@ -270,16 +265,7 @@ impl Error {
}
}
-impl error::Error for Error {
- fn description(&self) -> &str {
- match *self {
- Error::Transport(ref e) => TransportError::description(e),
- Error::Protocol(ref e) => ProtocolError::description(e),
- Error::Application(ref e) => ApplicationError::description(e),
- Error::User(ref e) => e.description(),
- }
- }
-}
+impl error::Error for Error {}
impl Debug for Error {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
@@ -370,6 +356,7 @@ impl TransportError {
/// I/O error categories.
///
/// This list may grow, and it is not recommended to match against it.
+#[non_exhaustive]
#[derive(Clone, Copy, Eq, Debug, PartialEq)]
pub enum TransportErrorKind {
/// Catch-all I/O error.
@@ -388,9 +375,9 @@ pub enum TransportErrorKind {
SizeLimit = 6,
}
-impl TransportError {
- fn description(&self) -> &str {
- match self.kind {
+impl Display for TransportError {
+ fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ let error_text = match self.kind {
TransportErrorKind::Unknown => "transport error",
TransportErrorKind::NotOpen => "not open",
TransportErrorKind::AlreadyOpen => "already open",
@@ -398,13 +385,9 @@ impl TransportError {
TransportErrorKind::EndOfFile => "end of file",
TransportErrorKind::NegativeSize => "negative size message",
TransportErrorKind::SizeLimit => "message too long",
- }
- }
-}
+ };
-impl Display for TransportError {
- fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
- write!(f, "{}", self.description())
+ write!(f, "{}", error_text)
}
}
@@ -434,24 +417,24 @@ impl From<io::Error> for Error {
| io::ErrorKind::ConnectionRefused
| io::ErrorKind::NotConnected => Error::Transport(TransportError {
kind: TransportErrorKind::NotOpen,
- message: err.description().to_owned(),
+ message: err.to_string(),
}),
io::ErrorKind::AlreadyExists => Error::Transport(TransportError {
kind: TransportErrorKind::AlreadyOpen,
- message: err.description().to_owned(),
+ message: err.to_string(),
}),
io::ErrorKind::TimedOut => Error::Transport(TransportError {
kind: TransportErrorKind::TimedOut,
- message: err.description().to_owned(),
+ message: err.to_string(),
}),
io::ErrorKind::UnexpectedEof => Error::Transport(TransportError {
kind: TransportErrorKind::EndOfFile,
- message: err.description().to_owned(),
+ message: err.to_string(),
}),
_ => {
Error::Transport(TransportError {
kind: TransportErrorKind::Unknown,
- message: err.description().to_owned(), // FIXME: use io error's debug string
+ message: err.to_string(), // FIXME: use io error's debug string
})
}
}
@@ -462,7 +445,7 @@ impl From<string::FromUtf8Error> for Error {
fn from(err: string::FromUtf8Error) -> Self {
Error::Protocol(ProtocolError {
kind: ProtocolErrorKind::InvalidData,
- message: err.description().to_owned(), // FIXME: use fmt::Error's debug string
+ message: err.to_string(), // FIXME: use fmt::Error's debug string
})
}
}
@@ -498,6 +481,7 @@ impl ProtocolError {
/// Runtime library error categories.
///
/// This list may grow, and it is not recommended to match against it.
+#[non_exhaustive]
#[derive(Clone, Copy, Eq, Debug, PartialEq)]
pub enum ProtocolErrorKind {
/// Catch-all runtime-library error.
@@ -518,9 +502,9 @@ pub enum ProtocolErrorKind {
DepthLimit = 6,
}
-impl ProtocolError {
- fn description(&self) -> &str {
- match self.kind {
+impl Display for ProtocolError {
+ fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ let error_text = match self.kind {
ProtocolErrorKind::Unknown => "protocol error",
ProtocolErrorKind::InvalidData => "bad data",
ProtocolErrorKind::NegativeSize => "negative message size",
@@ -528,13 +512,9 @@ impl ProtocolError {
ProtocolErrorKind::BadVersion => "invalid thrift version",
ProtocolErrorKind::NotImplemented => "not implemented",
ProtocolErrorKind::DepthLimit => "maximum skip depth reached",
- }
- }
-}
+ };
-impl Display for ProtocolError {
- fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
- write!(f, "{}", self.description())
+ write!(f, "{}", error_text)
}
}
@@ -589,6 +569,7 @@ impl ApplicationError {
/// Auto-generated or user-implemented code error categories.
///
/// This list may grow, and it is not recommended to match against it.
+#[non_exhaustive]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum ApplicationErrorKind {
/// Catch-all application error.
@@ -618,9 +599,9 @@ pub enum ApplicationErrorKind {
UnsupportedClientType = 10, // ??
}
-impl ApplicationError {
- fn description(&self) -> &str {
- match self.kind {
+impl Display for ApplicationError {
+ fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ let error_text = match self.kind {
ApplicationErrorKind::Unknown => "service error",
ApplicationErrorKind::UnknownMethod => "unknown service method",
ApplicationErrorKind::InvalidMessageType => "wrong message type received",
@@ -632,13 +613,9 @@ impl ApplicationError {
ApplicationErrorKind::InvalidTransform => "invalid transform",
ApplicationErrorKind::InvalidProtocol => "invalid protocol requested",
ApplicationErrorKind::UnsupportedClientType => "unsupported protocol client",
- }
- }
-}
+ };
-impl Display for ApplicationError {
- fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
- write!(f, "{}", self.description())
+ write!(f, "{}", error_text)
}
}