[ISSUE #5173]✨Add error handlingor uninitialized DefaultMQProducer in send_with_selector

[WaterWhisperer]

Which Issue(s) This PR Fixes(Closes)

Fixes #5173

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling in the message producer to gracefully return an error instead of crashing when the producer is not properly initialized.
  • Enhanced stability when sending messages through the message queue selector with an uninitialized producer state.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[rocketmq-rust-bot]

🔊@WaterWhisperer 🚀Thanks for your contribution🎉！

💡CodeRabbit(AI) will review your code first🔥！

Note

🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥.

[coderabbitai]

Walkthrough

This PR replaces two unwrap() calls on DefaultMQProducerImpl with explicit error handling, converting potential panics into RocketMQError::not_initialized() returns in the send-with-selector code path. Control flow remains unchanged when the inner value is present.

Changes

Cohort / File(s)	Summary
Error handling in producer send methods rocketmq-client/src/producer/default_mq_producer.rs	Replaced two unwrap() calls with ok_or(RocketMQError::not_initialized(...)) in selector-based and initial MQProducer send flows to gracefully handle uninitialized producer state instead of panicking.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• #5349: Replaces unwrap() on DefaultMQProducerImpl with explicit RocketMQError::not_initialized paths in the same file.
• #5366: Converts unwrap() to guarded error handling on optional DefaultMQProducerImpl for uninitialized producer detection.
• #5297: Changes the same file to replace unwrap() with RocketMQError::not_initialized in different send call sites.

Suggested labels

enhancement✨, Difficulty level/Easy, auto merge

Suggested reviewers

• SpaceXCN
• mxsm
• TeslaRustor

Poem

🐰 A producer once panicked with fear,
But now errors are handled with care!
No unwrap() despair, just ok_or grace,
The rabbit hops forward at a faster pace! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding error handling for uninitialized DefaultMQProducer in send_with_selector, which matches the actual code changes replacing unwrap() with proper error handling.
Linked Issues check	✅ Passed	The PR successfully implements the requirement from issue #5173 by converting unwrap() calls to ok_or() for proper error handling when DefaultMQProducerImpl is uninitialized in send_with_selector.
Out of Scope Changes check	✅ Passed	All changes are focused on error handling in send_with_selector for uninitialized DefaultMQProducer, directly aligned with issue #5173 with no out-of-scope modifications.

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

rocketmq-client/src/producer/default_mq_producer.rs (2)

1203-1231: Add test coverage for the changed method.

The test validates error handling for request_with_callback, but doesn't cover the specific method changed in this PR (send_with_selector). Consider adding a test that calls send_with_selector on an uninitialized producer to verify the fix.

🔎 Suggested test

#[tokio::test]
async fn send_with_selector_not_initialized() {
    // Arrange
    let mut producer = DefaultMQProducer {
        client_config: Default::default(),
        producer_config: Default::default(),
        default_mqproducer_impl: None,
    };
    let msg = Message {
        topic: "test-topic".into(),
        flag: 0,
        properties: Default::default(),
        body: None,
        compressed_body: None,
        transaction_id: None,
    };
    let selector = |_queues: &[MessageQueue], _msg: &dyn MessageTrait, _arg: &dyn std::any::Any| None;
    
    // Act
    let result = producer.send_with_selector(msg, selector, ()).await;
    
    // Assert
    assert!(result.is_err());
    let err = result.unwrap_err();
    match err {
        RocketMQError::NotInitialized(reason) => {
            assert!(reason.contains("not initialized"), "unexpected error message: {reason}");
        }
        other => panic!("Unexpected error: {other:?}"),
    }
}

---

1106-1106: Minor inconsistency in error message.

The error message here uses "DefaultMQProducerImpl is not initialized" while most other locations (including the newly changed line 820) use "DefaultMQProducerImpl not initialized" (without "is"). Consider standardizing for consistency.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88501a1 and 901f675.

📒 Files selected for processing (1)

• rocketmq-client/src/producer/default_mq_producer.rs

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-05-10T06:20:00.401Z

Learnt from: 578223592
Repo: mxsm/rocketmq-rust PR: 3240
File: rocketmq-namesrv/src/kvconfig/kvconfig_mananger.rs:69-71
Timestamp: 2025-05-10T06:20:00.401Z
Learning: In RocketMQ Rust, while dashmap is concurrent-safe, returning a direct reference to a DashMap can break encapsulation by allowing external code to modify the map without triggering persistence methods like `persist()`. Consider returning a read-only view, iterator methods, or a snapshot instead.

Applied to files:

• rocketmq-client/src/producer/default_mq_producer.rs

🧬 Code graph analysis (1)

rocketmq-client/src/producer/default_mq_producer.rs (1)

rocketmq-error/src/unified.rs (1)

• not_initialized (521-523)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Check (fmt + clippy)
• GitHub Check: auto-approve

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 37.26%. Comparing base (88501a1) to head (901f675).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...ocketmq-client/src/producer/default_mq_producer.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5396   +/-   ##
=======================================
  Coverage   37.26%   37.26%           
=======================================
  Files         803      803           
  Lines      108756   108755    -1     
=======================================
+ Hits        40523    40526    +3     
+ Misses      68233    68229    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rocketmq-rust-bot]

LGTM - All CI checks passed ✅