You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "adamcavendish (via GitHub)" <gi...@apache.org> on 2025/07/11 09:29:48 UTC
[PR] Rust: improve message status and timestamp handling [rocketmq-clients]
adamcavendish opened a new pull request, #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044
### Which Issue(s) This PR Fixes
Fixes #1043
### Brief Description
- Add handle_receive_message_status() function to properly handle different status codes
- Implement proper delivery timestamp processing for received messages
- Update client.rs and push_consumer.rs to use new status handling logic
- Add comprehensive status code mapping with appropriate error types
- Include MessageNotFound as valid status for receive operations
- Add unit tests for new status handling functionality
### How Did You Test This Change?
With the BUG reproduce steps, no more unhandled new warning logs matching these 3 kinds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #1043] [Rust] Improve message status and timestamp handling [rocketmq-clients]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Rust: improve message status and timestamp handling [rocketmq-clients]
Posted by "adamcavendish (via GitHub)" <gi...@apache.org>.
adamcavendish commented on PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044#issuecomment-3106386056
Requesting review from @EnableAsync @lizhimins @lizhanhui .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Rust: improve message status and timestamp handling [rocketmq-clients]
Posted by "adamcavendish (via GitHub)" <gi...@apache.org>.
adamcavendish commented on code in PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044#discussion_r2225086232
##########
rust/src/util.rs:
##########
@@ -223,6 +223,149 @@ pub fn handle_response_status(
Ok(())
}
+/// Handle status messages in receive message responses, similar to Java StatusChecker
+/// This function handles status codes appropriately based on the context
+pub fn handle_receive_message_status(
+ status: &Status,
+ operation: &'static str,
+) -> Result<(), ClientError> {
+ match status.code {
Review Comment:
Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Rust: improve message status and timestamp handling [rocketmq-clients]
Posted by "EnableAsync (via GitHub)" <gi...@apache.org>.
EnableAsync commented on code in PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044#discussion_r2224993652
##########
rust/src/util.rs:
##########
@@ -223,6 +223,149 @@ pub fn handle_response_status(
Ok(())
}
+/// Handle status messages in receive message responses, similar to Java StatusChecker
+/// This function handles status codes appropriately based on the context
+pub fn handle_receive_message_status(
+ status: &Status,
+ operation: &'static str,
+) -> Result<(), ClientError> {
+ match status.code {
Review Comment:
Excellent work on this PR!
I have a suggestion to make the new handle_receive_message_status function more concise and idiomatic by using Rust's match on the pb::Code enum with | (OR) patterns. This would greatly reduce boilerplate and improve readability.
Here's a quick example of what it could look like:
```rust
pub fn handle_receive_message_status(...) -> Result<(), ClientError> {
let code = Code::from_i32(status.code).unwrap_or(Code::Unspecified); // Simplified for example
match code {
// Success cases
Code::Ok | Code::MultipleResults | Code::MessageNotFound => Ok(()),
// Config errors
Code::BadRequest
| Code::IllegalTopic
| Code::IllegalConsumerGroup
// ... other config errors
=> Err(ClientError::new(ErrorKind::Config, "bad request", ...)),
// Server errors
Code::InternalError | Code::InternalServerError => {
Err(ClientError::new(ErrorKind::Server, "internal error", ...))
}
// ... other error groups
_ => Err(ClientError::new(ErrorKind::Server, "unhandled status", ...)),
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Rust: improve message status and timestamp handling [rocketmq-clients]
Posted by "EnableAsync (via GitHub)" <gi...@apache.org>.
EnableAsync commented on code in PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044#discussion_r2224993652
##########
rust/src/util.rs:
##########
@@ -223,6 +223,149 @@ pub fn handle_response_status(
Ok(())
}
+/// Handle status messages in receive message responses, similar to Java StatusChecker
+/// This function handles status codes appropriately based on the context
+pub fn handle_receive_message_status(
+ status: &Status,
+ operation: &'static str,
+) -> Result<(), ClientError> {
+ match status.code {
Review Comment:
Excellent work on this PR!
I have a suggestion to make the new handle_receive_message_status function more concise and idiomatic by using Rust's match on the pb::Code enum with | (OR) patterns. This would greatly reduce boilerplate and improve readability.
Here's a quick example of what it could look like:
```rust
pub fn handle_receive_message_status(...) -> Result<(), ClientError> {
let code = Code::from_i32(status.code).unwrap_or(Code::Unspecified); // Simplified for example
match code {
// Success cases
Code::Ok | Code::MultipleResults => Ok(()),
// Config errors
Code::BadRequest
| Code::IllegalTopic
| Code::IllegalConsumerGroup
// ... other config errors
=> Err(ClientError::new(ErrorKind::Config, "bad request", ...)),
// Server errors
Code::InternalError | Code::InternalServerError => {
Err(ClientError::new(ErrorKind::Server, "internal error", ...))
}
// ... other error groups
_ => Err(ClientError::new(ErrorKind::Server, "unhandled status", ...)),
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Rust: improve message status and timestamp handling [rocketmq-clients]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044#issuecomment-3106369762
## [Codecov](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `54.87179%` with `88 lines` in your changes missing coverage. Please review.
> Project coverage is 52.57%. Comparing base [(`4076e2b`)](https://app.codecov.io/gh/apache/rocketmq-clients/commit/4076e2b1d9979079c228aa52bfc8fa6dd874fa3b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`740411b`)](https://app.codecov.io/gh/apache/rocketmq-clients/commit/740411b8ae107aa6dc128a86458b9667f2ad39c8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 56 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [rust/src/util.rs](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?src=pr&el=tree&filepath=rust%2Fsrc%2Futil.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cnVzdC9zcmMvdXRpbC5ycw==) | 56.62% | [68 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [rust/src/client.rs](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?src=pr&el=tree&filepath=rust%2Fsrc%2Fclient.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cnVzdC9zcmMvY2xpZW50LnJz) | 55.55% | [7 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [rust/src/push\_consumer.rs](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?src=pr&el=tree&filepath=rust%2Fsrc%2Fpush_consumer.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cnVzdC9zcmMvcHVzaF9jb25zdW1lci5ycw==) | 27.27% | [7 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1044 +/- ##
============================================
- Coverage 53.26% 52.57% -0.70%
+ Complexity 651 648 -3
============================================
Files 208 207 -1
Lines 14303 13832 -471
Branches 5845 5540 -305
============================================
- Hits 7619 7272 -347
+ Misses 6308 6266 -42
+ Partials 376 294 -82
```
| [Flag](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [rust](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.55% <54.87%> (∅)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/rocketmq-clients/pull/1044?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Rust: improve message status and timestamp handling [rocketmq-clients]
Posted by "EnableAsync (via GitHub)" <gi...@apache.org>.
EnableAsync commented on PR #1044:
URL: https://github.com/apache/rocketmq-clients/pull/1044#issuecomment-3107255184
LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org