enh-5499:removed unwrap and added an error

[albertbolt1]

Which Issue(s) This PR Fixes(Closes)

Fixes #5499

Brief Description

removed unwrap and added an error

How Did You Test This Change?

ran the test cases to check if anything is breaking

Summary by CodeRabbit

• Bug Fixes
  • Enhanced error handling in the message producer to gracefully handle uninitialized state instead of crashing the application.

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

[coderabbitai]

Walkthrough

Replaces an unconditional unwrap() call on DefaultMQProducerImpl with explicit error handling via ok_or(), converting panic behavior to a graceful RocketMQError return when the inner implementation is uninitialized during the request() method invocation.

Changes

Cohort / File(s)	Summary
Error Handling for Uninitialized Producer rocketmq-client/src/producer/default_mq_producer.rs	Replaced unconditional unwrap() with ok_or() to return explicit RocketMQError when DefaultMQProducerImpl is not initialized in the request() path, preventing panic and enabling proper error propagation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A producer once panicked with dread,
When inner fields weren't properly fed,
But now with ok_or, we handle with grace,
Returning errors instead of lost face! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 accurately reflects the main change: removing an unwrap and replacing it with explicit error handling, which directly addresses the PR's primary objective.
Linked Issues check	✅ Passed	The PR successfully addresses issue #5499 by replacing an unconditional unwrap on DefaultMQProducerImpl with explicit error handling via ok_or, converting panics to proper RocketMQError returns.
Out of Scope Changes check	✅ Passed	The changes are narrowly scoped to the request() path in DefaultMQProducer, directly addressing the uninitialized producer error handling requirement without introducing unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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.}

[rocketmq-rust-bot]

🔊@albertbolt1 🚀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]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

948-948: Consider replacing remaining unwraps for consistency.

Several other methods still use unwrap() on default_mqproducer_impl:

• Line 948: send_batch()
• Line 1034: send_batch_with_callback_timeout()
• Line 1053: send_batch_to_queue_with_callback()
• Line 1072: send_batch_to_queue_with_callback_timeout()

For consistency and robustness, consider replacing these with the same ok_or() pattern used in the request() method to prevent potential panics.

♻️ Example fix for line 948

-        let result = self.default_mqproducer_impl.as_mut().unwrap().send(&mut batch).await?;
+        let result = self
+            .default_mqproducer_impl
+            .as_mut()
+            .ok_or(RocketMQError::not_initialized("DefaultMQProducerImpl not initialized"))?
+            .send(&mut batch)
+            .await?;

Apply similar changes to the other locations.

Also applies to: 1034-1034, 1053-1053, 1072-1072

📜 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 60e023b and cd5fd84.

📒 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 (534-536)

⏰ 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). (5)

• GitHub Check: Code Coverage
• GitHub Check: Build & Test (windows-latest)
• GitHub Check: Build & Test (ubuntu-latest)
• GitHub Check: Build & Test (macos-latest)
• GitHub Check: auto-approve

🔇 Additional comments (2)

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

1086-1091: LGTM! Proper error handling replaces unwrap.

The change correctly replaces a potential panic with explicit error handling using ok_or(), returning a RocketMQError::NotInitialized when the producer implementation is not initialized. This aligns with the PR objective and matches the pattern used consistently throughout the file.

---

1204-1329: Excellent test coverage for error handling.

The added tests thoroughly verify that the methods return a NotInitialized error when default_mqproducer_impl is None, ensuring the error handling behaves as expected.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 38.43%. Comparing base (60e023b) to head (cd5fd84).
⚠️ Report is 8 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    #5550   +/-   ##
=======================================
  Coverage   38.43%   38.43%           
=======================================
  Files         815      815           
  Lines      110551   110551           
=======================================
+ Hits        42490    42494    +4     
+ Misses      68061    68057    -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 ✅