[ISSUE #1482]♻️Refactor create MQClientErr replace with mq_client_err! macro🔥 #1483
Conversation
WalkthroughThe changes in this pull request primarily focus on refactoring error handling across various components of the RocketMQ client. The existing error handling patterns that utilized Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
rocketmq-rust-bot
added
AI review first
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
rocketmq-client/src/implementation/mq_admin_impl.rs (1)
Line range hint
123-124: Add TODO comment for unimplemented method.Consider adding a TODO comment explaining the intended implementation and tracking it with an issue.
pub async fn search_offset(&mut self, mq: &MessageQueue, timestamp: u64) -> Result<i64> { - unimplemented!("max_offset") + // TODO: Implement search_offset functionality + // See issue #<number> for implementation details + unimplemented!("search_offset") }rocketmq-client/src/base/validators.rs (1)
39-39: Consider enhancing error message clarity.The error message could be more specific about the requirements.
- return mq_client_err!("the specified group is blank"); + return mq_client_err!("the specified group cannot be empty or contain only whitespace");rocketmq-client/src/consumer/store/local_file_offset_store.rs (1)
103-106: Enhance error message clarityConsider including the deserialization error in the message for better debugging.
- Err(_) => mq_client_err!(format!( - "read local offset bak failed, content: {}", - content - )), + Err(e) => mq_client_err!(format!( + "Failed to deserialize backup local offset. Error: {}, Content: {}", + e, content + )),rocketmq-client/src/consumer/store/remote_broker_offset_store.rs (1)
320-320: Consider extracting common error handlingThis error message is identical to the one in
fetch_consume_offset_from_broker. Consider extracting it to a helper function.fn broker_not_found_error(broker_name: &str) -> crate::Result<()> { mq_client_err!(format!("broker not found, {}", broker_name)) }rocketmq-client/src/consumer/consumer_impl/pull_api_wrapper.rs (2)
363-363: Remove trailing comma in error messageThere's an unnecessary trailing comma in the format string.
- mq_client_err!(format!("The broker[{}] not exist", mq.get_broker_name(),)) + mq_client_err!(format!("The broker[{}] not exist", mq.get_broker_name()))
380-390: Consolidate duplicate error messagesThe same error message is repeated twice. Consider extracting it to a helper function.
fn filter_server_not_found_error(broker_addr: &str, topic: &str) -> crate::Result<CheetahString> { mq_client_err!(format!( "Find Filter Server Failed, Broker Addr: {},topic:{}", broker_addr, topic )) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (13)
rocketmq-client/src/base/validators.rs(5 hunks)rocketmq-client/src/consumer/consumer_impl/default_mq_push_consumer_impl.rs(6 hunks)rocketmq-client/src/consumer/consumer_impl/pull_api_wrapper.rs(4 hunks)rocketmq-client/src/consumer/consumer_impl/re_balance/rebalance_push_impl.rs(5 hunks)rocketmq-client/src/consumer/store/local_file_offset_store.rs(3 hunks)rocketmq-client/src/consumer/store/remote_broker_offset_store.rs(3 hunks)rocketmq-client/src/factory/mq_client_instance.rs(3 hunks)rocketmq-client/src/implementation/mq_admin_impl.rs(2 hunks)rocketmq-client/src/implementation/mq_client_api_impl.rs(5 hunks)rocketmq-client/src/producer/default_mq_producer.rs(2 hunks)rocketmq-client/src/producer/producer_impl/default_mq_producer_impl.rs(13 hunks)rocketmq-client/src/utils/message_util.rs(2 hunks)rocketmq/src/shutdown.rs(0 hunks)
💤 Files with no reviewable changes (1)
- rocketmq/src/shutdown.rs
🔇 Additional comments (24)
rocketmq-client/src/utils/message_util.rs (1)
75-81: LGTM! Error handling refactor looks good.
The error handling has been successfully migrated to use the mq_client_err! macro while maintaining the same error message and semantics.
rocketmq-client/src/implementation/mq_admin_impl.rs (1)
90-93: LGTM! Error handling refactor looks good.
The error handling has been successfully migrated to use the mq_client_err! macro while maintaining clear error messaging.
rocketmq-client/src/base/validators.rs (1)
39-39: LGTM! Error handling refactoring is consistent and well-implemented.
The migration to mq_client_err! macro has been done consistently across all validation methods while maintaining the original error semantics and codes.
Also applies to: 43-43, 47-51, 61-64, 71-74, 79-82, 86-92, 100-107, 116-116, 120-123, 127-131, 139-142, 149-149, 157-160, 169-172
rocketmq-client/src/consumer/store/local_file_offset_store.rs (1)
91-91: LGTM: Clean error handling refactor
The error handling has been cleanly refactored to use the mq_client_err! macro while maintaining clear error messaging.
rocketmq-client/src/consumer/store/remote_broker_offset_store.rs (1)
113-113: LGTM: Clear error message
The error handling is concise and provides the necessary broker information for debugging.
rocketmq-client/src/consumer/consumer_impl/pull_api_wrapper.rs (1)
302-308: LGTM: Well-formatted error message
The error message is comprehensive and well-formatted, including all necessary details about broker version and tag filtering support.
rocketmq-client/src/consumer/consumer_impl/re_balance/rebalance_push_impl.rs (1)
275-275: LGTM! Consistent error handling pattern.
The refactoring to use the mq_client_err! macro across all error cases in the compute_pull_from_where_with_exception method is well-implemented. The error handling is consistent and maintains proper error context.
Also applies to: 306-309, 321-324, 362-365, 370-373
rocketmq-client/src/producer/default_mq_producer.rs (1)
493-493: LGTM! Proper error handling with logging.
The error handling in the batch method appropriately logs the error details before using the mq_client_err! macro to return the error.
rocketmq-client/src/factory/mq_client_instance.rs (1)
333-336: LGTM! Clear and descriptive error handling.
The error handling using the mq_client_err! macro is well-implemented in both methods:
start: Provides clear context about the factory object statecheck_client_in_broker: Includes detailed explanation about potential version compatibility issues
Also applies to: 989-989
rocketmq-client/src/implementation/mq_client_api_impl.rs (4)
256-259: LGTM: Error handling refactored consistently
The error handling has been standardized using the mq_client_err! macro while preserving the original error information and behavior.
Also applies to: 262-265
725-728: LGTM: Error handling standardized
The error handling has been appropriately refactored to use the mq_client_err! macro while maintaining the same error information.
764-766: LGTM: Error handling refactored appropriately
The error handling has been standardized using the mq_client_err! macro while preserving the original error behavior.
1263-1266: LGTM: Error handling standardized consistently
The error handling has been appropriately refactored to use the mq_client_err! macro while maintaining the same error information.
rocketmq-client/src/producer/producer_impl/default_mq_producer_impl.rs (7)
429-429: LGTM: Error handling standardized appropriately
The error handling has been refactored to use the mq_client_err! macro while maintaining the original error information.
Also applies to: 433-433
874-896: LGTM: Error handling refactored consistently across multiple cases
The error handling has been standardized using the mq_client_err! macro across multiple error cases while preserving the original error behavior and information.
967-967: LGTM: Error handling standardized
The error handling has been appropriately refactored to use the mq_client_err! macro while maintaining the same error information.
1789-1795: LGTM: Error handling refactored appropriately
The error handling has been standardized using the mq_client_err! macro while preserving the original error information.
1870-1870: LGTM: Error handling standardized
The error handling has been appropriately refactored to use the mq_client_err! macro while maintaining the same error information.
2224-2229: LGTM: Error handling standardized consistently
The error handling has been appropriately refactored to use the mq_client_err! macro across multiple error cases while maintaining the original error information.
Also applies to: 2242-2242, 2245-2245, 2248-2252
2270-2274: LGTM: Error handling refactored appropriately
The error handling has been standardized using the mq_client_err! macro while preserving the original error information.
rocketmq-client/src/consumer/consumer_impl/default_mq_push_consumer_impl.rs (4)
358-358: LGTM: Error handling standardized consistently
The error handling has been appropriately refactored to use the mq_client_err! macro across multiple error cases while maintaining the original error information.
Also applies to: 361-361, 364-368
Line range hint 448-608: LGTM: Error handling refactored consistently across validation checks
The error handling has been standardized using the mq_client_err! macro across multiple validation checks while preserving the original error information and behavior.
673-673: LGTM: Error handling standardized
The error handling has been appropriately refactored to use the mq_client_err! macro while maintaining the same error information.
971-975: LGTM: Error handling refactored appropriately
The error handling has been standardized using the mq_client_err! macro while preserving the original error information.
rocketmq-rust-bot
added
approved
Which Issue(s) This PR Fixes(Closes)
Fixes #1482
Brief Description
How Did You Test This Change?
Summary by CodeRabbit
New Features
Shutdownstruct for managing shutdown signals with a Tokio broadcast channel.Shutdownstruct for checking shutdown status and receiving notifications.Bug Fixes
mq_client_err!.Documentation