This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new a573e3c396 [#14704] fix(jdbc): classify source refusals and keep
source messages literal (#14707)
a573e3c396 is described below
commit a573e3c39680ec673162dff401576a21ae21b7af
Author: Qi Yu <[email protected]>
AuthorDate: Sat Nov 7 10:15:51 2026 +0800
[#14704] fix(jdbc): classify source refusals and keep source messages
literal (#14707)
### What changes were proposed in this pull request?
- `JdbcExceptionConverter`, `StarRocksExceptionConverter` and the Paimon
`CatalogUtils` pass the source message through `"%s"`, so `%` in the
message stays literal.
- `PostgreSqlExceptionConverter` walks the cause chain to find the
SQLState, so a pool-wrapped connection failure is still classified. The
walk tracks visited exceptions by identity, so a cause cycle ends it.
- PostgreSQL SQLSTATE class 28 (rejected login) maps to
`ConnectionFailedException` (502).
- PostgreSQL SQLSTATE classes 22, 23 and 42 (other than the ones already
mapped) and 2BP01 map to `BadRequestException`. `BaseExceptionHandler`
returns 400 (`IllegalArgumentException`) for `BadRequestException`.
### Why are the changes needed?
Source refusals were returned as 500, and a source message containing
`%` was replaced by `UnknownFormatConversionException`. Clients could
not tell a user mistake or an outage from a server bug.
Fixes #14704
### Does this PR introduce _any_ user-facing change?
Yes, status codes only:
- A PostgreSQL rejected login or a pool-wrapped connection failure
returns 502 instead of 500.
- A request PostgreSQL refuses because of the request itself returns 400
instead of 500.
- Any `BadRequestException` that reaches a REST handler returns 400
instead of 500.
The source message is unchanged.
### How was this patch tested?
- New `TestJdbcExceptionConverter` (7 cases), extended
`TestPostgreSqlExceptionConverter` (30 cases, including cause cycles
with and without a SQLSTATE) and `TestExceptionHandlers` (11 cases)
pass.
- StarRocks and Paimon modules compile.
- With the production changes reverted, the new converter and handler
cases fail.
---
.../jdbc/converter/JdbcExceptionConverter.java | 13 ++-
.../jdbc/converter/TestJdbcExceptionConverter.java | 59 ++++++++++++++
.../converter/PostgreSqlExceptionConverter.java | 92 ++++++++++++++++------
.../TestPostgreSqlExceptionConverter.java | 71 +++++++++++++++++
.../converter/StarRocksExceptionConverter.java | 25 +++---
.../lakehouse/paimon/utils/CatalogUtils.java | 2 +-
.../server/web/rest/ExceptionHandlers.java | 8 ++
.../server/web/rest/TestExceptionHandlers.java | 20 +++++
8 files changed, 243 insertions(+), 47 deletions(-)
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcExceptionConverter.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcExceptionConverter.java
index d4a60c8dbf..466dd19664 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcExceptionConverter.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcExceptionConverter.java
@@ -38,25 +38,24 @@ public class JdbcExceptionConverter {
* @return The best attempt at a corresponding connector exception or
generic with the
* SQLException as the cause
*/
- @SuppressWarnings("FormatStringAnnotation")
public GravitinoRuntimeException toGravitinoException(final SQLException
sqlException) {
switch (sqlException.getErrorCode()) {
case 1007:
- return new SchemaAlreadyExistsException(sqlException,
sqlException.getMessage());
+ return new SchemaAlreadyExistsException(sqlException, "%s",
sqlException.getMessage());
case 1050:
- return new TableAlreadyExistsException(sqlException,
sqlException.getMessage());
+ return new TableAlreadyExistsException(sqlException, "%s",
sqlException.getMessage());
case 1008:
case 1049:
- return new NoSuchSchemaException(sqlException,
sqlException.getMessage());
+ return new NoSuchSchemaException(sqlException, "%s",
sqlException.getMessage());
case 1146:
case 1051:
- return new NoSuchTableException(sqlException,
sqlException.getMessage());
+ return new NoSuchTableException(sqlException, "%s",
sqlException.getMessage());
default:
if (sqlException.getMessage() != null
&& sqlException.getMessage().contains("Access denied")) {
- return new ConnectionFailedException(sqlException,
sqlException.getMessage());
+ return new ConnectionFailedException(sqlException, "%s",
sqlException.getMessage());
}
- return new GravitinoRuntimeException(sqlException,
sqlException.getMessage());
+ return new GravitinoRuntimeException(sqlException, "%s",
sqlException.getMessage());
}
}
}
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcExceptionConverter.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcExceptionConverter.java
new file mode 100644
index 0000000000..d90fa77760
--- /dev/null
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcExceptionConverter.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.catalog.jdbc.converter;
+
+import java.sql.SQLException;
+import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.apache.gravitino.exceptions.NoSuchSchemaException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/** Source messages are literal text, not format templates. */
+public class TestJdbcExceptionConverter {
+
+ @Test
+ void testAccessDeniedForAnyHostKeepsSourceMessage() {
+ SQLException failure =
+ new SQLException("Access denied for user 'gravitino'@'%' to database
'db'", "42000", 1044);
+ GravitinoRuntimeException error = new
JdbcExceptionConverter().toGravitinoException(failure);
+ Assertions.assertInstanceOf(ConnectionFailedException.class, error);
+ Assertions.assertSame(failure, error.getCause());
+ Assertions.assertEquals(failure.getMessage(), error.getMessage());
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1007, 1050, 1049, 1146, 0})
+ void testEveryBranchKeepsPercentSignsLiteral(int errorCode) {
+ SQLException failure = new SQLException("object 'a%'b' failed at 100%",
"HY000", errorCode);
+ GravitinoRuntimeException error = new
JdbcExceptionConverter().toGravitinoException(failure);
+ Assertions.assertSame(failure, error.getCause());
+ Assertions.assertEquals(failure.getMessage(), error.getMessage());
+ }
+
+ @Test
+ void testMissingSchemaIsStillClassified() {
+ Assertions.assertInstanceOf(
+ NoSuchSchemaException.class,
+ new JdbcExceptionConverter()
+ .toGravitinoException(new SQLException("Unknown database 'x%'",
"42000", 1049)));
+ }
+}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlExceptionConverter.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlExceptionConverter.java
index e9c721fcab..255479a97f 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlExceptionConverter.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlExceptionConverter.java
@@ -18,8 +18,14 @@
*/
package org.apache.gravitino.catalog.postgresql.converter;
+import com.google.common.collect.ImmutableSet;
import java.sql.SQLException;
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Set;
+import javax.annotation.Nullable;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
+import org.apache.gravitino.exceptions.BadRequestException;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
@@ -37,6 +43,7 @@ public class PostgreSqlExceptionConverter extends
JdbcExceptionConverter {
private static final String INVALID_SCHEMA = "3F000";
private static final String UNDEFINED_TABLE = "42P01";
private static final String INSUFFICIENT_PRIVILEGE = "42501";
+ private static final String DEPENDENT_OBJECTS_STILL_EXIST = "2BP01";
/**
* SQLSTATE '08' is the class code of connection exceptions See <a
@@ -45,35 +52,68 @@ public class PostgreSqlExceptionConverter extends
JdbcExceptionConverter {
*/
private static final String CONNECTION_EXCEPTION = "08";
+ /** SQLSTATE '28' is the class code of invalid authorization specification,
a rejected login. */
+ private static final String INVALID_AUTHORIZATION = "28";
+
+ /**
+ * SQLSTATE classes of requests that PostgreSQL refuses because of the
request itself: '22' data
+ * exception, '23' integrity constraint violation and '42' syntax error or
access rule violation.
+ */
+ private static final Set<String> INVALID_REQUEST_CLASSES =
ImmutableSet.of("22", "23", "42");
+
@Override
public GravitinoRuntimeException toGravitinoException(SQLException se) {
- if (null != se.getSQLState()) {
- switch (se.getSQLState()) {
- case INSUFFICIENT_PRIVILEGE:
- return new ForbiddenException(se, "%s", se.getMessage());
- case DUPLICATE_DATABASE:
- case DUPLICATE_SCHEMA:
- return new SchemaAlreadyExistsException(se, "%s", se.getMessage());
- case DUPLICATE_TABLE:
- return new TableAlreadyExistsException(se, "%s", se.getMessage());
- case INVALID_SCHEMA_NAME:
- case INVALID_SCHEMA:
- return new NoSuchSchemaException(se, "%s", se.getMessage());
- case UNDEFINED_TABLE:
- return new NoSuchTableException(se, "%s", se.getMessage());
- default:
- {
- if (se.getSQLState().startsWith(CONNECTION_EXCEPTION)) {
- return new ConnectionFailedException(se, "%s", se.getMessage());
- }
- return new GravitinoRuntimeException(se, "%s", se.getMessage());
- }
- }
- } else {
- if (se.getMessage() != null && se.getMessage().contains("password
authentication failed")) {
- return new ConnectionFailedException(se, "%s", se.getMessage());
+ // A pool wraps the driver's exception, for example DBCP's "Cannot create
+ // PoolableConnectionFactory", so the SQLSTATE that names the cause can be
on a nested
+ // exception. Track visited exceptions by identity so a cause cycle (A ->
B -> A) ends the walk.
+ Set<Throwable> visited = Collections.newSetFromMap(new
IdentityHashMap<>());
+ for (Throwable t = se; t != null && visited.add(t); t = t.getCause()) {
+ if (t instanceof SQLException) {
+ GravitinoRuntimeException converted =
+ convertBySqlState(se, ((SQLException) t).getSQLState());
+ if (converted != null) {
+ return converted;
+ }
}
- return new GravitinoRuntimeException(se, "%s", se.getMessage());
+ }
+
+ if (se.getMessage() != null && se.getMessage().contains("password
authentication failed")) {
+ return new ConnectionFailedException(se, "%s", se.getMessage());
+ }
+ return new GravitinoRuntimeException(se, "%s", se.getMessage());
+ }
+
+ @Nullable
+ private static GravitinoRuntimeException convertBySqlState(
+ SQLException se, @Nullable String sqlState) {
+ if (sqlState == null) {
+ return null;
+ }
+
+ switch (sqlState) {
+ case INSUFFICIENT_PRIVILEGE:
+ return new ForbiddenException(se, "%s", se.getMessage());
+ case DUPLICATE_DATABASE:
+ case DUPLICATE_SCHEMA:
+ return new SchemaAlreadyExistsException(se, "%s", se.getMessage());
+ case DUPLICATE_TABLE:
+ return new TableAlreadyExistsException(se, "%s", se.getMessage());
+ case INVALID_SCHEMA_NAME:
+ case INVALID_SCHEMA:
+ return new NoSuchSchemaException(se, "%s", se.getMessage());
+ case UNDEFINED_TABLE:
+ return new NoSuchTableException(se, "%s", se.getMessage());
+ case DEPENDENT_OBJECTS_STILL_EXIST:
+ return new BadRequestException(se, "%s", se.getMessage());
+ default:
+ if (sqlState.startsWith(CONNECTION_EXCEPTION)
+ || sqlState.startsWith(INVALID_AUTHORIZATION)) {
+ return new ConnectionFailedException(se, "%s", se.getMessage());
+ }
+ if (sqlState.length() >= 2 &&
INVALID_REQUEST_CLASSES.contains(sqlState.substring(0, 2))) {
+ return new BadRequestException(se, "%s", se.getMessage());
+ }
+ return null;
}
}
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlExceptionConverter.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlExceptionConverter.java
index e45514e637..042eb19185 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlExceptionConverter.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlExceptionConverter.java
@@ -19,8 +19,11 @@
package org.apache.gravitino.catalog.postgresql.converter;
import java.sql.SQLException;
+import java.time.Duration;
+import org.apache.gravitino.exceptions.BadRequestException;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -76,4 +79,72 @@ public class TestPostgreSqlExceptionConverter {
new SQLException("connection or protocol failure", state)));
}
}
+
+ @Test
+ void testPoolWrappedConnectionFailureUsesNestedSqlState() {
+ SQLException driverFailure = new SQLException("The connection attempt
failed.", "08001");
+ SQLException poolFailure =
+ new SQLException("Cannot create PoolableConnectionFactory (connect
failed)", driverFailure);
+ Exception error = new
PostgreSqlExceptionConverter().toGravitinoException(poolFailure);
+ Assertions.assertInstanceOf(ConnectionFailedException.class, error);
+ Assertions.assertSame(poolFailure, error.getCause());
+ Assertions.assertEquals(poolFailure.getMessage(), error.getMessage());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"28P01", "28000"})
+ void testRejectedLoginIsConnectionFailure(String state) {
+ SQLException failure = new SQLException("password authentication failed
for user", state);
+ Exception error = new
PostgreSqlExceptionConverter().toGravitinoException(failure);
+ Assertions.assertInstanceOf(ConnectionFailedException.class, error);
+ Assertions.assertSame(failure, error.getCause());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"2BP01", "42601", "42703", "42804", "23502",
"23505", "22003", "22P02"})
+ void testRequestsRefusedBySourceAreBadRequests(String state) {
+ SQLException failure = new SQLException("source refused the request",
state);
+ Exception error = new
PostgreSqlExceptionConverter().toGravitinoException(failure);
+ Assertions.assertInstanceOf(BadRequestException.class, error);
+ Assertions.assertSame(failure, error.getCause());
+ Assertions.assertEquals(failure.getMessage(), error.getMessage());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"XX000", "53300", "57014"})
+ void testServerSideFailuresRemainGeneric(String state) {
+ SQLException failure = new SQLException("server side failure", state);
+ Exception error = new
PostgreSqlExceptionConverter().toGravitinoException(failure);
+ Assertions.assertEquals(GravitinoRuntimeException.class, error.getClass());
+ }
+
+ @Test
+ void testCauseCycleWithoutSqlStateEndsTheWalk() {
+ SQLException failure = new SQLException("source failure");
+ RuntimeException wrapper = new RuntimeException("wrapper");
+ failure.initCause(wrapper);
+ wrapper.initCause(failure);
+ Exception error = convertWithinTimeout(failure);
+ Assertions.assertEquals(GravitinoRuntimeException.class, error.getClass());
+ Assertions.assertSame(failure, error.getCause());
+ }
+
+ @Test
+ void testCauseCycleStillFindsNestedSqlState() {
+ SQLException failure = new SQLException("pool failure");
+ SQLException driverFailure = new SQLException("permission denied",
"42501");
+ failure.initCause(driverFailure);
+ driverFailure.initCause(failure);
+ Exception error = convertWithinTimeout(failure);
+ Assertions.assertInstanceOf(ForbiddenException.class, error);
+ Assertions.assertSame(failure, error.getCause());
+ }
+
+ // A regression loops without checking interruption, so the conversion must
not run on the test
+ // thread.
+ private static Exception convertWithinTimeout(SQLException failure) {
+ return Assertions.assertTimeoutPreemptively(
+ Duration.ofSeconds(10),
+ () -> new
PostgreSqlExceptionConverter().toGravitinoException(failure));
+ }
}
diff --git
a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/converter/StarRocksExceptionConverter.java
b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/converter/StarRocksExceptionConverter.java
index af57eabd98..7cc46244c3 100644
---
a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/converter/StarRocksExceptionConverter.java
+++
b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/converter/StarRocksExceptionConverter.java
@@ -50,40 +50,39 @@ public class StarRocksExceptionConverter extends
JdbcExceptionConverter {
static final int CODE_PARTITION_ALREADY_EXISTS = 1517;
static final int CODE_GENERIC_ERROR = 5064;
- @SuppressWarnings("FormatStringAnnotation")
@Override
public GravitinoRuntimeException toGravitinoException(SQLException se) {
int errorCode = se.getErrorCode();
switch (errorCode) {
case CODE_DATABASE_EXISTS:
- return new SchemaAlreadyExistsException(se, se.getMessage());
+ return new SchemaAlreadyExistsException(se, "%s", se.getMessage());
case CODE_TABLE_EXISTS:
- return new TableAlreadyExistsException(se, se.getMessage());
+ return new TableAlreadyExistsException(se, "%s", se.getMessage());
case CODE_DATABASE_NOT_EXISTS:
case CODE_UNKNOWN_DATABASE:
case CODE_UNKNOWN_DATABASE_2:
- return new NoSuchSchemaException(se, se.getMessage());
+ return new NoSuchSchemaException(se, "%s", se.getMessage());
case CODE_NO_SUCH_TABLE:
case CODE_NO_SUCH_TABLE_2:
- return new NoSuchTableException(se, se.getMessage());
+ return new NoSuchTableException(se, "%s", se.getMessage());
case CODE_UNAUTHORIZED:
- return new UnauthorizedException(se, se.getMessage());
+ return new UnauthorizedException(se, "%s", se.getMessage());
case CODE_NO_SUCH_COLUMN:
- return new NoSuchColumnException(se, se.getMessage());
+ return new NoSuchColumnException(se, "%s", se.getMessage());
case CODE_DELETE_NON_EXISTING_PARTITION:
- return new NoSuchPartitionException(se, se.getMessage());
+ return new NoSuchPartitionException(se, "%s", se.getMessage());
case CODE_PARTITION_ALREADY_EXISTS:
- return new PartitionAlreadyExistsException(se, se.getMessage());
+ return new PartitionAlreadyExistsException(se, "%s", se.getMessage());
case CODE_GENERIC_ERROR:
if (se.getMessage() != null && se.getMessage().contains("Unknown
properties")) {
- return new IllegalPropertyException(se, se.getMessage());
+ return new IllegalPropertyException(se, "%s", se.getMessage());
}
- return new GravitinoRuntimeException(se, se.getMessage());
+ return new GravitinoRuntimeException(se, "%s", se.getMessage());
default:
if (se.getMessage() != null && se.getMessage().contains("Access
denied")) {
- return new ConnectionFailedException(se, se.getMessage());
+ return new ConnectionFailedException(se, "%s", se.getMessage());
}
- return new GravitinoRuntimeException(se, se.getMessage());
+ return new GravitinoRuntimeException(se, "%s", se.getMessage());
}
}
}
diff --git
a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/utils/CatalogUtils.java
b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/utils/CatalogUtils.java
index aae1317095..ab3d8f5bd6 100644
---
a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/utils/CatalogUtils.java
+++
b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/utils/CatalogUtils.java
@@ -134,7 +134,7 @@ public class CatalogUtils {
} catch (RuntimeException e) {
if (e.getCause() instanceof SQLException
&& e.getCause().getMessage().contains("Access denied")) {
- throw new ConnectionFailedException(e, e.getMessage());
+ throw new ConnectionFailedException(e, "%s", e.getMessage());
}
throw e;
}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
index 07032081a7..eb7efd56d2 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
@@ -23,6 +23,7 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.exceptions.AlreadyExistsException;
+import org.apache.gravitino.exceptions.BadRequestException;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
@@ -1253,6 +1254,13 @@ public class ExceptionHandlers {
String errorMsg =
getBaseErrorMsg(formattedObject, op.name(), formattedParent,
getErrorMsg(e));
+ // A request the backend refuses because of the request itself (for
example an invalid
+ // definition or a drop blocked by dependent objects) is a client error,
not a server error.
+ if (e instanceof BadRequestException) {
+ LOG.warn(errorMsg, e);
+ return
Utils.illegalArguments(IllegalArgumentException.class.getSimpleName(),
errorMsg, e);
+ }
+
// A configured dependency that is temporarily unavailable is not an
internal Gravitino error:
// surface it as 503 Service Unavailable so callers can tell an outage
from a server bug.
if (e instanceof ServiceUnavailableException) {
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
index 706202fadd..52f85485e1 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
@@ -22,6 +22,7 @@ import java.util.List;
import javax.ws.rs.core.Response;
import org.apache.gravitino.dto.responses.ErrorConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.exceptions.BadRequestException;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.InUseException;
import org.apache.gravitino.exceptions.NoSuchJobTemplateException;
@@ -34,6 +35,25 @@ import org.junit.jupiter.api.Test;
public class TestExceptionHandlers {
+ @Test
+ void testBackendRefusedRequestReturnsBadRequest() {
+ BadRequestException failure =
+ new BadRequestException("cannot drop table t because other objects
depend on it");
+ try (Response tableResponse =
+ ExceptionHandlers.handleTableException(OperationType.DROP, "t",
"schema", failure);
+ Response schemaResponse =
+ ExceptionHandlers.handleSchemaException(
+ OperationType.CREATE, "schema", "catalog", failure)) {
+ for (Response response : List.of(tableResponse, schemaResponse)) {
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
error.getCode());
+
Assertions.assertEquals(IllegalArgumentException.class.getSimpleName(),
error.getType());
+ Assertions.assertTrue(error.getMessage().contains("other objects
depend on it"));
+ }
+ }
+ }
+
@Test
void testConcurrentActiveJobReturnsConflict() {
try (Response response =
