[ISSUE #10685] Tolerate malformed timer properties in gRPC converter

[Aias00]

Which Issue(s) This PR Fixes

Fixes #10685

Brief Description

GrpcConverter.buildSystemProperties previously parsed PROPERTY_TIMER_DELAY_SEC and PROPERTY_TIMER_DELIVER_MS with Long.parseLong directly. If either optional timer property was malformed, converting a Broker MessageExt to the gRPC v2 Message response could throw NumberFormatException.

This PR adds safe parsing for those timer properties. Valid values still populate delivery_timestamp; malformed values are ignored with a warning so the message conversion can continue.

How Did You Test This Change?

mvn -pl proxy -Dtest=GrpcConverterTest -DfailIfNoTests=false test

Result: BUILD SUCCESS, Tests run: 6, Failures: 0, Errors: 0, Skipped: 0.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds safe parsing for optional timer properties (PROPERTY_TIMER_DELAY_SEC and PROPERTY_TIMER_DELIVER_MS) in GrpcConverter.buildSystemProperties to tolerate malformed values instead of throwing NumberFormatException.

Findings

• [Info] GrpcConverter.java:254 — The new parseLongMessageProperty helper is well-designed: returns null for missing properties, catches NumberFormatException for malformed values, and logs with sufficient context (topic, msgId, property name, value) for debugging.
• [Info] GrpcConverter.java:191-200 — The refactored delivery_timestamp block is cleaner. The precedence (delay-sec first, then deliver-ms) is preserved from the original code.
• [Info] GrpcConverterTest.java — Good test coverage: 5 new test cases covering valid delay, invalid delay, valid deliver, invalid deliver, and both missing. Tests use Timestamps.toMillis for assertion which is correct.

Suggestions

• [Info] Consider making parseLongMessageProperty private instead of protected if external subclassing is not intended. The test class is in the same package, so private with package-private test access or @VisibleForTesting would also work. This is a minor style point and not blocking.

Overall: Clean defensive improvement. The change prevents a potential runtime exception from propagating to gRPC clients when broker messages contain malformed timer metadata.

---

Automated review by github-manager-bot

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.24%. Comparing base (00e45b8) to head (c5f8888).

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10686      +/-   ##
=============================================
- Coverage      48.31%   48.24%   -0.07%     
+ Complexity     13511    13486      -25     
=============================================
  Files           1380     1380              
  Lines         101091   101099       +8     
  Branches       13101    13102       +1     
=============================================
- Hits           48844    48780      -64     
- Misses         46285    46337      +52     
- Partials        5962     5982      +20     

☔ View full report in Codecov by Harness.
📢 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.

[RockteMQ-AI]

Review by github-manager-bot (Re-review)

PR #10686: [ISSUE #10685] Tolerate malformed timer properties in gRPC converter
Re-review scope: New commit c5f8888 — "Narrow timer property parser visibility"

Changes Since Last Review

• parseLongMessageProperty visibility narrowed (private helper) — correct encapsulation since it is only used within GrpcConverter
• Simplified the delivery timestamp logic: removed intermediate variables, made the flow more linear

Assessment

Correctness — The refactoring preserves the original behavior while improving readability.
Design — Making the helper private is the right call — it is an internal implementation detail.

Overall: LGTM — Clean refactoring, no concerns.

---

Automated re-review by github-manager-bot