This is an automated email from the ASF dual-hosted git repository.
roryqi 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 a0c3e63503 [#13686] fix(iceberg-rest): Preserve
WebApplicationException HTTP status (#13687)
a0c3e63503 is described below
commit a0c3e635039a27e14d3e02376e5c9166ac33c1c5
Author: MaSai <[email protected]>
AuthorDate: Thu Oct 8 16:06:33 2026 +0800
[#13686] fix(iceberg-rest): Preserve WebApplicationException HTTP status
(#13687)
### What changes were proposed in this pull request?
- In `IcebergExceptionMapper`, preserve the HTTP status from JAX-RS
`WebApplicationException` instead of defaulting unmapped exceptions to
500.
- Keep `WebApplicationException` instances in
`convertToIcebergException` so auth paths that call `getErrorCode` stay
consistent.
- Add unit coverage for 406/405/415 and an Iceberg config integration
test with `Accept: application/vnd.gravitino.v1+json`.
### Why are the changes needed?
Content-negotiation failures are client errors (406), not server
failures (500). Returning 500 misleads clients and monitoring.
Fix: #13686
### Does this PR introduce _any_ user-facing change?
Yes. Iceberg REST responses for unacceptable `Accept` / unsupported
media type / disallowed method now return the correct HTTP status and
matching Iceberg `error.code` (e.g. 406), instead of 500.
### How was this patch tested?
- `./gradlew :iceberg:iceberg-rest-server:test --tests
org.apache.gravitino.iceberg.service.TestIcebergExceptionMapper --tests
org.apache.gravitino.iceberg.service.rest.TestIcebergConfig -PskipITs`
---------
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 b22fe5b407..913dad38d9 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;
@@ -86,14 +87,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();
}
/**
@@ -118,7 +131,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);
@@ -138,8 +151,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 b9192d31db..67fa0a3b52 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"));
