fuyou001 commented on PR #10689: URL: https://github.com/apache/rocketmq/pull/10689#issuecomment-5116882868
Suggestion: consider changing `TopicRouteWrapper.getMasterAddr` to return `Optional<String>` instead of a nullable `String`. There are only three direct call sites, and `Optional` would force each one to handle a missing broker explicitly. - In the ordered-route branch of `MessageQueueSelector.buildWrite`, use an empty result to log and skip the malformed/unknown broker entry. - In the normal `QueueData` write branch, use an empty result to skip entries without a master address. - In `ClusterTopicRouteService.getBrokerAddr`, convert an empty result into the existing explicit broker-not-found exception rather than returning `null`. The third point is important because `ClusterTopicRouteService.getBrokerAddr` has many indirect callers. Returning `null` can bypass the exception handling in `resolveBrokerAddr` and `resolveBrokerAddrInReceiptHandle`, allowing a null address to reach the remoting client instead of producing `INVALID_BROKER_NAME` or `INVALID_RECEIPT_HANDLE`. Please also add regression tests asserting that an unknown broker produces `Optional.empty()`, while `ClusterTopicRouteService.getBrokerAddr` still fails with the expected domain exception. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
