Skip to content

[ISSUE #1462]🔥Add macro for create client broker error🚀 #1465

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

Merged
merged 1 commit into from
Nov 30, 2024
Merged

[ISSUE #1462]🔥Add macro for create client broker error🚀 #1465

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 44 additions & 12 deletions rocketmq-client/src/client_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,26 @@
}
}

#[macro_export]
macro_rules! client_broker_err {
// Handle errors with a custom ResponseCode and formatted string
($response_code:expr, $error_message:expr, $broker_addr:expr) => {{
std::result::Result::Err($crate::client_error::MQClientError::MQClientBrokerError(
$crate::client_error::MQBrokerErr::new_with_broker(
$response_code as i32,
$error_message,
$broker_addr,
),
))
}};
// Handle errors without a ResponseCode, using only the error message
($response_code:expr, $error_message:expr) => {{
std::result::Result::Err($crate::client_error::MQClientError::MQClientBrokerError(
$crate::client_error::MQBrokerErr::new($response_code as i32, $error_message),
))
}};
}
Comment on lines +104 to +122
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add documentation and support format strings

The macro implementation could be improved in several ways:

  1. Missing documentation comments explaining usage and examples
  2. Unlike mq_client_err! and request_timeout_err!, this macro doesn't support format strings with multiple arguments

Consider adding documentation and extending the macro like this:

 #[macro_export]
+/// Creates a broker-related error result
+/// 
+/// # Examples
+/// 
+/// ```
+/// let err = client_broker_err!(404, "Error: {} - {}", "Not Found", "broker1");
+/// let err = client_broker_err!(404, "Not Found");
+/// ```
 macro_rules! client_broker_err {
+    // Handle errors with formatted string and broker address
+    ($response_code:expr, $fmt:expr, $($arg:expr),*, $broker_addr:expr) => {{
+        let formatted_msg = format!($fmt, $($arg),*);
+        std::result::Result::Err($crate::client_error::MQClientError::MQClientBrokerError(
+            $crate::client_error::MQBrokerErr::new_with_broker(
+                $response_code as i32,
+                formatted_msg,
+                $broker_addr,
+            ),
+        ))
+    }};
     // Handle errors with a custom ResponseCode and formatted string
     ($response_code:expr, $error_message:expr, $broker_addr:expr) => {{

Committable suggestion skipped: line range outside the PR's diff.


#[derive(Error, Debug)]
#[error("{message}")]
pub struct ClientErr {
Expand Down Expand Up @@ -232,6 +252,30 @@
use super::*;
use crate::client_error;

#[test]
fn client_broker_err_with_response_code_and_broker_formats_correctly() {
let result: std::result::Result<(), client_error::MQClientError> =
client_broker_err!(404, "Error: {}", "127.0.0.1");
assert!(result.is_err());
if let Err(MQClientError::MQClientBrokerError(err)) = result {
assert_eq!(err.response_code(), 404);
assert_eq!(err.error_message().unwrap(), "Error: {}");
assert_eq!(err.broker_addr().unwrap(), "127.0.0.1");
}

Check warning on line 264 in rocketmq-client/src/client_error.rs

View check run for this annotation

Codecov / codecov/patch

rocketmq-client/src/client_error.rs#L264 

Added line #L264 was not covered by tests
}

#[test]
fn client_broker_err_with_response_code_formats_correctly() {
let result: std::result::Result<(), client_error::MQClientError> =
client_broker_err!(404, "Error: not found");
assert!(result.is_err());
if let Err(MQClientError::MQClientBrokerError(err)) = result {
assert_eq!(err.response_code(), 404);
assert_eq!(err.error_message().unwrap(), "Error: not found");
assert!(err.broker_addr().is_none());
}

Check warning on line 276 in rocketmq-client/src/client_error.rs

View check run for this annotation

Codecov / codecov/patch

rocketmq-client/src/client_error.rs#L276 

Added line #L276 was not covered by tests
}

#[test]
fn request_timeout_err_with_response_code_formats_correctly() {
let result: std::result::Result<(), client_error::MQClientError> =
Expand All @@ -240,8 +284,6 @@
if let Err(MQClientError::RequestTimeoutError(err)) = result {
assert_eq!(err.response_code(), 408);
assert_eq!(err.error_message().unwrap(), "Request timed out");
} else {
panic!("Expected MQClientError::RequestTimeoutError");
}
}

Expand All @@ -252,8 +294,6 @@
if let Err(MQClientError::RequestTimeoutError(err)) = result {
assert_eq!(err.response_code(), -1);
assert_eq!(err.error_message().unwrap(), "Timeout error");
} else {
panic!("Expected MQClientError::RequestTimeoutError");
}
}

Expand All @@ -265,8 +305,6 @@
if let Err(MQClientError::RequestTimeoutError(err)) = result {
assert_eq!(err.response_code(), 504);
assert_eq!(err.error_message().unwrap(), "Error: Gateway - Timeout");
} else {
panic!("Expected MQClientError::RequestTimeoutError");
}
}

Expand All @@ -278,8 +316,6 @@
if let Err(MQClientError::MQClientErr(err)) = result {
assert_eq!(err.response_code(), 404);
assert_eq!(err.error_message().unwrap(), "Error: not found");
} else {
panic!("Expected MQClientError::MQClientErr");
}
}

Expand All @@ -290,8 +326,6 @@
if let Err(MQClientError::MQClientErr(err)) = result {
assert_eq!(err.response_code(), -1);
assert_eq!(err.error_message().unwrap(), "simple error");
} else {
panic!("Expected MQClientError::MQClientErr");
}
}

Expand All @@ -306,8 +340,6 @@
err.error_message().unwrap(),
"Error: internal - server error"
);
} else {
panic!("Expected MQClientError::MQClientErr");
}
}

Expand Down
Loading