C0urante commented on a change in pull request #10530:
URL: https://github.com/apache/kafka/pull/10530#discussion_r612518884
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
##########
@@ -369,9 +370,31 @@ else if (serverConnector != null &&
serverConnector.getHost() != null && serverC
else if (serverConnector != null && serverConnector.getPort() > 0)
builder.port(serverConnector.getPort());
- log.info("Advertised URI: {}", builder.build());
+ URI uri = builder.build();
+ validateUriHost(uri);
+ log.info("Advertised URI: {}", uri);
- return builder.build();
+ return uri;
+ }
+
+ /**
+ * Parses the uri and throws a more definitive error
+ * when the internal node to node communication can't happen due to an
invalid host name.
+ */
+ private void validateUriHost(URI uri) {
+ if (uri.getHost() == null) {
+ String host = Utils.getHost(uri.getAuthority());
+ String errorMsg = "Invalid host=" + host + ", in url=" +
uri.toString();
Review comment:
If `host` is null (due to a parsing failure in `Utils::getHost`), won't
this lead to a similarly-confusing error message for the user? I'm not sure
`Invalid host=null, in url ...` would be easy to decipher, and although it's
unclear if we could ever trigger that code path at the moment, it may be worth
considering in case later refactoring of this and other classes makes it
possible.
Perhaps something like "Could not parse host from advertised URL <url>"
would help shed more light here?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
