geyanggang commented on code in PR #13769:
URL: https://github.com/apache/gravitino/pull/13769#discussion_r4247174896
##########
catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogOperations.java:
##########
@@ -244,16 +244,67 @@ public void testConnection(NameIdentifier catalogIdent) {
try {
databaseOperation.listDatabases();
} catch (ConnectionFailedException e) {
- throw e;
+ // Already classified as a connection failure (for example a MySQL
access-denied or a
+ // PostgreSQL SQLSTATE 08 error). Re-wrap it with the host context so
every connection failure
+ // satisfies the documented "names the host" contract, preserving the
original cause.
+ throw new ConnectionFailedException(
+ e.getCause() != null ? e.getCause() : e,
+ "Failed to connect to JDBC catalog at %s: %s",
+ hostPortFromJdbcUrl(jdbcUrl),
+ e.getMessage());
} catch (GravitinoRuntimeException e) {
if (e.getClass() == GravitinoRuntimeException.class && e.getCause()
instanceof SQLException) {
throw new ConnectionFailedException(
- e.getCause(), "Failed to connect to JDBC catalog: %s",
e.getMessage());
+ e.getCause(),
+ "Failed to connect to JDBC catalog at %s: %s",
+ hostPortFromJdbcUrl(jdbcUrl),
+ e.getMessage());
}
throw e;
}
}
+ /**
+ * Extracts {@code host:port} from a {@code
jdbc:subprotocol://host:port/...} URL for use in a
+ * connection-failure message. Returns {@code "unknown"} whenever the
host/port cannot be located
+ * (including a URL with no {@code ://} authority) rather than the raw URL,
because the raw URL
+ * can carry credentials (for example {@code
jdbc:postgresql:db?password=...}) that must not leak
+ * into the exception, the REST response, or the server logs. Never throws,
so message formatting
+ * cannot mask the original connection failure.
+ *
+ * @param url the JDBC URL stored for this catalog
+ * @return a {@code host:port} description, or {@code "unknown"} when it
cannot be extracted
+ */
+ private static String hostPortFromJdbcUrl(String url) {
+ if (StringUtils.isBlank(url)) {
+ return "unknown";
+ }
+ int schemeEnd = url.indexOf("://");
+ if (schemeEnd < 0) {
+ return "unknown";
+ }
+ int authorityStart = schemeEnd + "://".length();
+ // The authority ends at the first '/', '?', or ';' following it.
+ int authorityEnd = url.length();
+ for (int i = authorityStart; i < url.length(); i++) {
+ char c = url.charAt(i);
+ if (c == '/' || c == '?' || c == ';') {
+ authorityEnd = i;
+ break;
+ }
+ }
+ String authority = url.substring(authorityStart, authorityEnd);
+ if (StringUtils.isBlank(authority)) {
+ return "unknown";
+ }
+ // Strip any user-info ("user@host:port"); keep only the host:port part.
+ int at = authority.lastIndexOf('@');
+ if (at >= 0) {
+ authority = authority.substring(at + 1);
+ }
+ return StringUtils.isBlank(authority) ? "unknown" : authority;
Review Comment:
Fixed.
##########
catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java:
##########
@@ -216,6 +216,10 @@ void
testExistingCatalogConnectionWithUnreachableEndpoint() throws SQLException
JdbcConfig.JDBC_DRIVER.getKey(),
MYSQL_CONTAINER.getDriverClassName(TEST_DB_NAME));
catalogProperties.put(JdbcConfig.USERNAME.getKey(),
MYSQL_CONTAINER.getUsername());
catalogProperties.put(JdbcConfig.PASSWORD.getKey(),
MYSQL_CONTAINER.getPassword());
+ // This test registers a catalog pointing at an unreachable endpoint on
purpose, so it can then
+ // assert testConnection fails on the EXISTING catalog. It must opt out of
create-time
+ // verification; the rejection by the default create-time verification is
covered separately.
Review Comment:
Done
--
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]
