This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 7fa3d90568 [Cherry-pick to branch-1.3] [#13686] fix(iceberg-rest):
Preserve WebApplicationException HTTP status (#13687) (#13701)
7fa3d90568 is described below
commit 7fa3d90568298b3cf7f4d9f1c1517178108fc822
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Oct 9 10:06:05 2026 +0800
[Cherry-pick to branch-1.3] [#13686] fix(iceberg-rest): Preserve
WebApplicationException HTTP status (#13687) (#13701)
**Cherry-pick Information:**
- Original commit: a0c3e635039a27e14d3e02376e5c9166ac33c1c5
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: MaSai <[email protected]>
Co-authored-by: Cursor <[email protected]>
---
.../iceberg/service/IcebergExceptionMapper.java | 28 ++++++++++++++++------
.../service/TestIcebergExceptionMapper.java | 24 +++++++++++++++++++
.../iceberg/service/rest/TestIcebergConfig.java | 19 +++++++++++++++
.../rest/TestIcebergNamespaceOperations.java | 5 ++--
4 files changed, 67 insertions(+), 9 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
index 2b1f1fd788..d12e6e8019 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.iceberg.service;
import com.google.common.collect.ImmutableMap;
import java.util.Map;
import javax.ws.rs.NotFoundException;
+import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import javax.ws.rs.ext.ExceptionMapper;
@@ -83,14 +84,26 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Throwable> {
.build();
/**
- * Returns the HTTP status code for the given exception based on the Iceberg
REST spec.
+ * Returns the HTTP status code for the given exception.
+ *
+ * <ol>
+ * <li>Iceberg / Gravitino business exceptions from {@link
#EXCEPTION_ERROR_CODES}
+ * <li>JAX-RS {@link WebApplicationException} that already carries an HTTP
status
+ * <li>Unexpected failures default to 500
+ * </ol>
*
* @param ex the exception
- * @return the HTTP status code, defaulting to 500 for unmapped exceptions
+ * @return the HTTP status code
*/
public static int getErrorCode(Exception ex) {
- return EXCEPTION_ERROR_CODES.getOrDefault(
- ex.getClass(), Status.INTERNAL_SERVER_ERROR.getStatusCode());
+ Integer code = EXCEPTION_ERROR_CODES.get(ex.getClass());
+ if (code != null) {
+ return code;
+ }
+ if (ex instanceof WebApplicationException) {
+ return ((WebApplicationException) ex).getResponse().getStatus();
+ }
+ return Status.INTERNAL_SERVER_ERROR.getStatusCode();
}
/**
@@ -115,7 +128,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Throwable> {
|| e instanceof ValidationException) {
return new BadRequestException("%s", message);
}
- if (EXCEPTION_ERROR_CODES.containsKey(e.getClass())) {
+ if (EXCEPTION_ERROR_CODES.containsKey(e.getClass()) || e instanceof
WebApplicationException) {
return e;
}
return new ServiceFailureException("%s", message);
@@ -135,8 +148,9 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Throwable> {
public static Response toRESTResponse(Throwable ex) {
ServerHealth.getInstance().recordFailure(ex);
int status =
- EXCEPTION_ERROR_CODES.getOrDefault(
- ex.getClass(), Status.INTERNAL_SERVER_ERROR.getStatusCode());
+ ex instanceof Exception
+ ? getErrorCode((Exception) ex)
+ : Status.INTERNAL_SERVER_ERROR.getStatusCode();
if (status == Status.INTERNAL_SERVER_ERROR.getStatusCode()) {
LOG.error("Iceberg REST server unexpected failure:", ex);
} else {
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
index 4226dfdf5e..1ce5ebc6ba 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
@@ -18,7 +18,10 @@
*/
package org.apache.gravitino.iceberg.service;
+import javax.ws.rs.NotAcceptableException;
+import javax.ws.rs.NotAllowedException;
import javax.ws.rs.NotFoundException;
+import javax.ws.rs.NotSupportedException;
import javax.ws.rs.core.Response;
import org.apache.gravitino.exceptions.TokenExpiredException;
import org.apache.gravitino.server.web.ServerHealth;
@@ -57,6 +60,9 @@ public class TestIcebergExceptionMapper {
checkExceptionStatus(new NoSuchTableException(""), 404);
checkExceptionStatus(new NoSuchIcebergTableException(""), 404);
checkExceptionStatus(new UnsupportedOperationException(""), 406);
+ checkExceptionStatus(new NotAcceptableException(), 406);
+ checkExceptionStatus(new NotAllowedException("GET"), 405);
+ checkExceptionStatus(new NotSupportedException("unsupported content
type"), 415);
checkExceptionStatus(new AlreadyExistsException(""), 409);
checkExceptionStatus(new CommitFailedException(""), 409);
checkExceptionStatus(new UnprocessableEntityException(""), 422);
@@ -65,6 +71,24 @@ public class TestIcebergExceptionMapper {
checkExceptionStatus(new RuntimeException(), 500);
}
+ @Test
+ public void testWebApplicationExceptionPreservesStatusInErrorBody() {
+ try (Response response = icebergExceptionMapper.toResponse(new
NotAcceptableException())) {
+ Assertions.assertEquals(406, response.getStatus());
+ ErrorResponse entity = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(406, entity.code());
+ Assertions.assertEquals("NotAcceptableException", entity.type());
+ }
+ }
+
+ @Test
+ public void testConvertToIcebergExceptionKeepsWebApplicationException() {
+ NotAcceptableException original = new NotAcceptableException();
+ Exception converted =
IcebergExceptionMapper.convertToIcebergException(original);
+ Assertions.assertSame(original, converted);
+ Assertions.assertEquals(406,
IcebergExceptionMapper.getErrorCode(converted));
+ }
+
/** Checks that errors retain their type and nested causes in the response.
*/
@Test
public void testErrorsRetainTypeAndCause() {
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
index f60eebf36f..3f4fd84506 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
@@ -25,7 +25,9 @@ import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
+import org.apache.gravitino.iceberg.service.IcebergObjectMapperProvider;
import org.apache.iceberg.rest.responses.ConfigResponse;
+import org.apache.iceberg.rest.responses.ErrorResponse;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
@@ -106,6 +108,23 @@ public class TestIcebergConfig extends IcebergTestBase {
Assertions.assertEquals(404, response.getStatus());
}
+ @Test
+ public void testConfigUnacceptableAcceptReturns406() {
+ // Gravitino media types are for /api/*, not Iceberg REST. Content
negotiation must return
+ // 406 (and the Iceberg error body code must match), not a misleading 500.
+ try (Response resp =
+ target(IcebergRestTestUtil.CONFIG_PATH)
+ .register(IcebergObjectMapperProvider.class)
+ .request()
+ .accept("application/vnd.gravitino.v1+json")
+ .get()) {
+ Assertions.assertEquals(Response.Status.NOT_ACCEPTABLE.getStatusCode(),
resp.getStatus());
+ ErrorResponse error = resp.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(406, error.code());
+ Assertions.assertEquals("NotAcceptableException", error.type());
+ }
+ }
+
@Test
public void testConfigEndpointsContainViewOperations() {
String warehouseName = IcebergRestTestUtil.PREFIX;
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
index dab90563fc..fb491d260a 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
@@ -198,8 +198,9 @@ public class TestIcebergNamespaceOperations extends
IcebergNamespaceTestBase {
Assertions.assertTrue(
dummyEventListener.popPostEvent() instanceof
IcebergDropNamespaceFailureEvent);
- // jersery route failed
- verifyDropNamespaceFail(500, Namespace.of(""));
+ // Empty namespace encodes to a path Jersey cannot bind for DELETE → 405
Method Not Allowed
+ // (previously mis-mapped to 500 by IcebergExceptionMapper).
+ verifyDropNamespaceFail(405, Namespace.of(""));
verifyCreateNamespaceSucc(Namespace.of("drop_foo3", "a"));
verifyDropNamespaceFail(404, Namespace.of("drop_foo3", "b"));
