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 6d01a90355 [Cherry-pick to branch-1.3] [#12975] fix(core): Preserve
errors thrown by PrincipalUtils.doAs (#12976) (#13064)
6d01a90355 is described below
commit 6d01a9035589dceed7d14863feaddc41561d38c2
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 10 15:31:05 2026 +0800
[Cherry-pick to branch-1.3] [#12975] fix(core): Preserve errors thrown by
PrincipalUtils.doAs (#12976) (#13064)
### What changes were proposed in this pull request?
Cherry-pick #12976 (commit 7478ab48e64aad1e57bac0ee0cf1cd86d9f13b8f) to
`branch-1.3`.
Rethrow the original `Error` from `PrincipalUtils.doAs` after logging
it, instead of wrapping it in a `RuntimeException`. Preserve error
diagnostics in REST responses and add regression tests.
The cherry-pick conflicts in `LanceRESTService` and
`TestFilesetOperations` have been resolved against the `branch-1.3`
APIs.
### Why are the changes needed?
Wrapping fatal JVM errors such as `OutOfMemoryError` in a
`RuntimeException` changes their classification and prevents callers and
error handlers from recognizing the actual failure.
Fix: #12975
### Does this PR introduce _any_ user-facing change?
Yes. Errors thrown by privileged actions retain their original type
instead of being exposed as `RuntimeException`. REST error responses
also preserve the original error diagnostics.
### How was this patch tested?
- `./gradlew :lance:lance-rest-server:test --tests
org.apache.gravitino.lance.service.TestLanceExceptionMapper -PskipITs`
- `./gradlew :server:test --tests
org.apache.gravitino.server.web.rest.TestFilesetOperations -PskipITs`
---------
Co-authored-by: roryqi <[email protected]>
Co-authored-by: roryqi <[email protected]>
---
.../gravitino/dto/responses/ErrorResponse.java | 18 ++-
.../gravitino/json/TestResponseJsonSerDe.java | 23 ++++
.../org/apache/gravitino/utils/PrincipalUtils.java | 7 +-
.../apache/gravitino/utils/TestPrincipalUtils.java | 124 +++++++++++++++++++++
.../iceberg/service/IcebergExceptionMapper.java | 12 +-
.../iceberg/service/TestIcebergErrorHandling.java | 122 ++++++++++++++++++++
.../service/TestIcebergExceptionMapper.java | 32 +++++-
.../apache/gravitino/lance/LanceRESTService.java | 2 +
.../lance/service/LanceExceptionMapper.java | 8 +-
.../lance/service/TestLanceExceptionMapper.java | 90 +++++++++++++++
.../apache/gravitino/server/GravitinoServer.java | 2 +
.../server/web/mapper/ErrorExceptionMapper.java | 47 ++++++++
.../web/mapper/TestErrorExceptionMapper.java | 52 +++++++++
.../server/web/rest/TestFilesetOperations.java | 42 +++++--
14 files changed, 553 insertions(+), 28 deletions(-)
diff --git
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
index a47d4cbb80..3927a743b6 100644
--- a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
+++ b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
@@ -203,11 +203,21 @@ public class ErrorResponse extends BaseResponse {
* @return The new instance.
*/
public static ErrorResponse internalError(String message, Throwable
throwable) {
+ return internalError(RuntimeException.class.getSimpleName(), message,
throwable);
+ }
+
+ /**
+ * Creates an internal error response with an explicit error type.
+ *
+ * @param type The type of the error.
+ * @param message The message of the error.
+ * @param throwable The throwable that caused the error, if available.
+ * @return The new error response.
+ */
+ public static ErrorResponse internalError(
+ String type, String message, @Nullable Throwable throwable) {
return new ErrorResponse(
- ErrorConstants.INTERNAL_ERROR_CODE,
- RuntimeException.class.getSimpleName(),
- message,
- getStackTrace(throwable));
+ ErrorConstants.INTERNAL_ERROR_CODE, type, message,
getStackTrace(throwable));
}
/**
diff --git
a/common/src/test/java/org/apache/gravitino/json/TestResponseJsonSerDe.java
b/common/src/test/java/org/apache/gravitino/json/TestResponseJsonSerDe.java
index 9729fbf3d6..1bbf5baea6 100644
--- a/common/src/test/java/org/apache/gravitino/json/TestResponseJsonSerDe.java
+++ b/common/src/test/java/org/apache/gravitino/json/TestResponseJsonSerDe.java
@@ -35,6 +35,29 @@ import org.junit.jupiter.api.Test;
public class TestResponseJsonSerDe {
+ /**
+ * Checks that custom internal error types and causes survive JSON
serialization.
+ *
+ * @throws JsonProcessingException If serialization fails.
+ */
+ @Test
+ public void testInternalErrorRetainsTypeAndCause() throws
JsonProcessingException {
+ Error error = new NoClassDefFoundError("catalog class");
+ error.initCause(new ClassNotFoundException("missing dependency"));
+ ErrorResponse response =
+ ErrorResponse.internalError("NoClassDefFoundError", "Server error",
error);
+ String json = JsonUtils.objectMapper().writeValueAsString(response);
+ ErrorResponse restored = JsonUtils.objectMapper().readValue(json,
ErrorResponse.class);
+ Assertions.assertEquals(response, restored);
+ Assertions.assertEquals("NoClassDefFoundError", restored.getType());
+ Assertions.assertTrue(
+ String.join("\n", restored.getStack())
+ .contains("Caused by: java.lang.ClassNotFoundException: missing
dependency"));
+ Assertions.assertEquals(
+ "RuntimeException", ErrorResponse.internalError("existing behavior",
error).getType());
+ Assertions.assertNull(ErrorResponse.internalError("Error", "No stack",
null).getStack());
+ }
+
@Test
public void testBaseResponseSerDe() throws JsonProcessingException {
BaseResponse response = new BaseResponse();
diff --git a/core/src/main/java/org/apache/gravitino/utils/PrincipalUtils.java
b/core/src/main/java/org/apache/gravitino/utils/PrincipalUtils.java
index bd3210c5e7..a324964f11 100644
--- a/core/src/main/java/org/apache/gravitino/utils/PrincipalUtils.java
+++ b/core/src/main/java/org/apache/gravitino/utils/PrincipalUtils.java
@@ -52,12 +52,13 @@ public class PrincipalUtils {
subject.getPrincipals().add(principal);
return Subject.doAs(subject, action);
} catch (PrivilegedActionException pae) {
+ LOG.error("doAs method encountered an exception", pae);
Throwable cause = pae.getCause();
Throwables.propagateIfPossible(cause, Exception.class);
throw new RuntimeException("doAs method encountered an unexpected
exception", pae);
- } catch (Error t) {
- LOG.warn("doAs method encountered an unexpected error", t);
- throw new RuntimeException("doAs method encountered an unexpected
exception", t);
+ } catch (Error error) {
+ LOG.error("doAs method encountered an unexpected error", error);
+ throw error;
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/utils/TestPrincipalUtils.java
b/core/src/test/java/org/apache/gravitino/utils/TestPrincipalUtils.java
index a2a285737d..d05d51842b 100644
--- a/core/src/test/java/org/apache/gravitino/utils/TestPrincipalUtils.java
+++ b/core/src/test/java/org/apache/gravitino/utils/TestPrincipalUtils.java
@@ -19,9 +19,20 @@
package org.apache.gravitino.utils;
+import java.security.PrivilegedActionException;
+import java.util.ArrayList;
+import java.util.List;
import org.apache.gravitino.UserPrincipal;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
public class TestPrincipalUtils {
@@ -52,4 +63,117 @@ public class TestPrincipalUtils {
return null;
});
}
+
+ @Test
+ public void testErrorIsPropagated() {
+ UserPrincipal principal = new UserPrincipal("testErrorIsPropagated");
+ AssertionError error = new AssertionError("test error");
+
+ AssertionError thrown =
+ Assertions.assertThrows(
+ AssertionError.class,
+ () ->
+ PrincipalUtils.doAs(
+ principal,
+ () -> {
+ throw error;
+ }));
+
+ Assertions.assertSame(error, thrown);
+ }
+
+ /** Checks that checked exceptions retain their identity and cause. */
+ @Test
+ public void testCheckedExceptionIsPropagated() {
+ Exception cause = new Exception("root cause");
+ Exception exception = new Exception("checked failure", cause);
+ Exception thrown =
+ Assertions.assertThrows(
+ Exception.class,
+ () ->
+ PrincipalUtils.doAs(
+ new UserPrincipal("test"),
+ () -> {
+ throw exception;
+ }));
+ Assertions.assertSame(exception, thrown);
+ Assertions.assertSame(cause, thrown.getCause());
+ }
+
+ /** Checks that runtime exceptions retain their identity and cause. */
+ @Test
+ public void testRuntimeExceptionIsPropagated() {
+ Exception cause = new Exception("root cause");
+ RuntimeException exception = new IllegalArgumentException("invalid
argument", cause);
+ RuntimeException thrown =
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () ->
+ PrincipalUtils.doAs(
+ new UserPrincipal("test"),
+ () -> {
+ throw exception;
+ }));
+ Assertions.assertSame(exception, thrown);
+ Assertions.assertSame(cause, thrown.getCause());
+ }
+
+ /** Checks that caught failures are logged at ERROR with the throwable. */
+ @Test
+ public void testFailuresAreLoggedWithThrowable() {
+ LoggerContext context =
+ (LoggerContext)
LogManager.getContext(PrincipalUtils.class.getClassLoader(), false);
+ Configuration configuration = context.getConfiguration();
+ String loggerName = PrincipalUtils.class.getName();
+ LoggerConfig previousConfig = configuration.getLoggers().get(loggerName);
+ Appender appender = Mockito.mock(Appender.class);
+ Mockito.when(appender.getName()).thenReturn("principalUtilsCapture");
+ Mockito.when(appender.isStarted()).thenReturn(true);
+ List<LogEvent> events = new ArrayList<>();
+ Mockito.doAnswer(
+ invocation -> {
+ events.add(((LogEvent) invocation.getArgument(0)).toImmutable());
+ return null;
+ })
+ .when(appender)
+ .append(Mockito.any(LogEvent.class));
+ LoggerConfig loggerConfig = new LoggerConfig(loggerName, Level.ERROR,
false);
+ loggerConfig.addAppender(appender, Level.ERROR, null);
+ configuration.removeLogger(loggerName);
+ configuration.addLogger(loggerName, loggerConfig);
+ context.updateLoggers();
+ try {
+ Error error = new AssertionError("request error");
+ Assertions.assertThrows(
+ Error.class,
+ () ->
+ PrincipalUtils.doAs(
+ new UserPrincipal("test"),
+ () -> {
+ throw error;
+ }));
+ Exception exception = new Exception("checked failure", new
Exception("root cause"));
+ Assertions.assertThrows(
+ Exception.class,
+ () ->
+ PrincipalUtils.doAs(
+ new UserPrincipal("test"),
+ () -> {
+ throw exception;
+ }));
+ Assertions.assertEquals(2, events.size());
+ Assertions.assertEquals(Level.ERROR, events.get(0).getLevel());
+ Assertions.assertSame(error, events.get(0).getThrown());
+ Assertions.assertEquals(Level.ERROR, events.get(1).getLevel());
+ Throwable logged = events.get(1).getThrown();
+ Assertions.assertInstanceOf(PrivilegedActionException.class, logged);
+ Assertions.assertSame(exception, logged.getCause());
+ } finally {
+ configuration.removeLogger(loggerName);
+ if (previousConfig != null) {
+ configuration.addLogger(loggerName, previousConfig);
+ }
+ context.updateLoggers();
+ }
+ }
}
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 daaf1db3aa..e7c9cb9546 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
@@ -50,7 +50,7 @@ import org.slf4j.LoggerFactory;
// Referred from Apache Iceberg's EXCEPTION_ERROR_CODES implementation
// core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
@Provider
-public class IcebergExceptionMapper implements ExceptionMapper<Exception> {
+public class IcebergExceptionMapper implements ExceptionMapper<Throwable> {
private static final Logger LOG =
LoggerFactory.getLogger(IcebergExceptionMapper.class);
@@ -120,8 +120,14 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
return new ServiceFailureException("%s", message);
}
+ /**
+ * Maps an uncaught throwable to an Iceberg REST error response.
+ *
+ * @param ex the failure raised while processing the request
+ * @return the error response, defaulting to HTTP 500 for unmapped failures
+ */
@Override
- public Response toResponse(Exception ex) {
+ public Response toResponse(Throwable ex) {
return toRESTResponse(ex);
}
@@ -130,7 +136,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
EXCEPTION_ERROR_CODES.getOrDefault(
ex.getClass(), Status.INTERNAL_SERVER_ERROR.getStatusCode());
if (status == Status.INTERNAL_SERVER_ERROR.getStatusCode()) {
- LOG.warn("Iceberg REST server unexpected exception:", ex);
+ LOG.error("Iceberg REST server unexpected failure:", ex);
} else {
LOG.info(
"Iceberg REST server error maybe caused by user request, response
http status: {}, exception: {}, exception message: {}",
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergErrorHandling.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergErrorHandling.java
new file mode 100644
index 0000000000..4c8ab70eb7
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergErrorHandling.java
@@ -0,0 +1,122 @@
+/*
+ * 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.iceberg.service;
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.core.Application;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.rest.RESTUtils;
+import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.iceberg.rest.responses.ErrorResponse;
+import org.glassfish.jersey.jackson.JacksonFeature;
+import org.glassfish.jersey.server.ResourceConfig;
+import org.glassfish.jersey.test.JerseyTest;
+import org.glassfish.jersey.test.TestProperties;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests Iceberg HTTP responses for errors raised on the request path. */
+public class TestIcebergErrorHandling extends JerseyTest {
+
+ /** Simulates direct failures and failures inside the REST doAs boundary. */
+ @Path("failure")
+ public static class FailureResource {
+
+ /**
+ * Executes a request with an optional failure.
+ *
+ * @param mode whether to fail directly, within doAs, or return a
successful response
+ * @return the request response
+ */
+ @GET
+ @Path("{mode}")
+ public Response request(@PathParam("mode") String mode) {
+ if ("healthy".equals(mode)) {
+ return Response.ok().build();
+ }
+ Error failure = new NoClassDefFoundError("catalog class");
+ failure.initCause(new ClassNotFoundException("missing dependency"));
+ if ("direct".equals(mode)) {
+ throw failure;
+ }
+ try {
+ return PrincipalUtils.doAs(
+ new UserPrincipal("test"),
+ () -> {
+ throw failure;
+ });
+ } catch (Exception e) {
+ return IcebergExceptionMapper.toRESTResponse(e);
+ }
+ }
+ }
+
+ /**
+ * Registers the same error and JSON mappers as the Iceberg REST service.
+ *
+ * @return the test application
+ */
+ @Override
+ protected Application configure() {
+ try {
+ forceSet(
+ TestProperties.CONTAINER_PORT,
String.valueOf(RESTUtils.findAvailablePort(2000, 3000)));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ return new ResourceConfig()
+ .register(FailureResource.class)
+ .register(IcebergExceptionMapper.class)
+ .register(IcebergObjectMapperProvider.class)
+ .register(JacksonFeature.class);
+ }
+
+ /**
+ * Verifies diagnostics survive both request paths and later requests can
still succeed.
+ *
+ * @throws IOException if the JSON response cannot be parsed
+ */
+ @Test
+ public void testErrorResponsesAndSubsequentRequests() throws IOException {
+ for (String mode : new String[] {"direct", "do-as"}) {
+ try (Response response =
+ target("failure/" +
mode).request(MediaType.APPLICATION_JSON_TYPE).get()) {
+ Assertions.assertEquals(500, response.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
response.getMediaType());
+ ErrorResponse error =
+ IcebergObjectMapper.getInstance()
+ .readValue(response.readEntity(String.class),
ErrorResponse.class);
+ Assertions.assertEquals(500, error.code());
+ Assertions.assertEquals("NoClassDefFoundError", error.type());
+ Assertions.assertEquals("catalog class", error.message());
+ Assertions.assertTrue(
+ String.join("\n", error.stack())
+ .contains("Caused by: java.lang.ClassNotFoundException:
missing dependency"));
+ }
+ try (Response response = target("failure/healthy").request().get()) {
+ Assertions.assertEquals(200, response.getStatus());
+ }
+ }
+ }
+}
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 eef610f6b0..354f4d26bd 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
@@ -33,17 +33,13 @@ import org.apache.iceberg.exceptions.NotAuthorizedException;
import org.apache.iceberg.exceptions.ServiceUnavailableException;
import org.apache.iceberg.exceptions.UnprocessableEntityException;
import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.rest.responses.ErrorResponse;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
public class TestIcebergExceptionMapper {
private final IcebergExceptionMapper icebergExceptionMapper = new
IcebergExceptionMapper();
- private void checkExceptionStatus(Exception exception, int statusCode) {
- Response response = icebergExceptionMapper.toResponse(exception);
- Assertions.assertEquals(statusCode, response.getStatus());
- }
-
@Test
public void testIcebergExceptionMapper() {
checkExceptionStatus(new IllegalArgumentException(""), 400);
@@ -65,4 +61,30 @@ public class TestIcebergExceptionMapper {
checkExceptionStatus(new ServiceUnavailableException(""), 503);
checkExceptionStatus(new RuntimeException(), 500);
}
+
+ /** Checks that errors retain their type and nested causes in the response.
*/
+ @Test
+ public void testErrorsRetainTypeAndCause() {
+ for (Error error :
+ new Error[] {
+ new OutOfMemoryError("Metaspace"), new StackOverflowError(),
+ new NoClassDefFoundError("catalog class"), new
AssertionError("assertion")
+ }) {
+ error.initCause(new IllegalStateException("root cause"));
+ try (Response response = icebergExceptionMapper.toResponse(error)) {
+ Assertions.assertEquals(500, response.getStatus());
+ ErrorResponse entity = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(error.getClass().getSimpleName(),
entity.type());
+ Assertions.assertEquals(error.getMessage(), entity.message());
+ Assertions.assertTrue(
+ String.join("\n", entity.stack())
+ .contains("Caused by: java.lang.IllegalStateException: root
cause"));
+ }
+ }
+ }
+
+ private void checkExceptionStatus(Exception exception, int statusCode) {
+ Response response = icebergExceptionMapper.toResponse(exception);
+ Assertions.assertEquals(statusCode, response.getStatus());
+ }
}
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
index 98f86aaae5..1bf493db80 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
@@ -30,6 +30,7 @@ import
org.apache.gravitino.auxiliary.GravitinoAuxiliaryService;
import org.apache.gravitino.lance.common.config.LanceConfig;
import org.apache.gravitino.lance.common.ops.LanceNamespaceBackend;
import org.apache.gravitino.lance.common.ops.NamespaceWrapper;
+import org.apache.gravitino.lance.service.LanceExceptionMapper;
import org.apache.gravitino.listener.EventBus;
import org.apache.gravitino.listener.api.event.EventSource;
import org.apache.gravitino.metrics.MetricsSystem;
@@ -80,6 +81,7 @@ public class LanceRESTService implements
GravitinoAuxiliaryService {
ResourceConfig resourceConfig = new ResourceConfig();
resourceConfig.register(JacksonFeature.class);
resourceConfig.packages(LANCE_REST_SPEC_PACKAGE);
+ resourceConfig.register(LanceExceptionMapper.class);
resourceConfig.register(
new AbstractBinder() {
@Override
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
index 2078b75fe6..47d0b7521a 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
@@ -42,11 +42,11 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Provider
-public class LanceExceptionMapper implements ExceptionMapper<Exception> {
+public class LanceExceptionMapper implements ExceptionMapper<Throwable> {
private static final Logger LOG =
LoggerFactory.getLogger(LanceExceptionMapper.class);
- public static Response toRESTResponse(String instance, Exception ex) {
+ public static Response toRESTResponse(String instance, Throwable ex) {
LanceNamespaceException lanceException =
ex instanceof LanceNamespaceException
? (LanceNamespaceException) ex
@@ -61,11 +61,11 @@ public class LanceExceptionMapper implements
ExceptionMapper<Exception> {
}
@Override
- public Response toResponse(Exception ex) {
+ public Response toResponse(Throwable ex) {
return toRESTResponse("", ex);
}
- private static LanceNamespaceException toLanceNamespaceException(String
instance, Exception ex) {
+ private static LanceNamespaceException toLanceNamespaceException(String
instance, Throwable ex) {
if (ex instanceof NoSuchTableException) {
return new TableNotFoundException(ex.getMessage(), getStackTrace(ex),
instance);
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
new file mode 100644
index 0000000000..fd3d98ded6
--- /dev/null
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
@@ -0,0 +1,90 @@
+/*
+ * 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.lance.service;
+
+import java.io.IOException;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.core.Application;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.rest.RESTUtils;
+import org.glassfish.jersey.jackson.JacksonFeature;
+import org.glassfish.jersey.server.ResourceConfig;
+import org.glassfish.jersey.test.JerseyTest;
+import org.glassfish.jersey.test.TestProperties;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.lance.namespace.model.ErrorResponse;
+
+/** Tests for {@link LanceExceptionMapper}. */
+public class TestLanceExceptionMapper extends JerseyTest {
+
+ /** A resource that raises an error outside the operation-level exception
handlers. */
+ @Path("error")
+ public static class ErrorResource {
+
+ /**
+ * Raises an assertion error.
+ *
+ * @return never returns normally
+ */
+ @GET
+ public String fail() {
+ AssertionError error = new AssertionError("assertion failure");
+ error.initCause(new IllegalStateException("root cause"));
+ throw error;
+ }
+ }
+
+ /**
+ * Configures the test resource and Lance exception mapper.
+ *
+ * @return the test application
+ */
+ @Override
+ protected Application configure() {
+ try {
+ forceSet(
+ TestProperties.CONTAINER_PORT,
String.valueOf(RESTUtils.findAvailablePort(2000, 3000)));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ return new ResourceConfig()
+ .register(ErrorResource.class)
+ .register(LanceExceptionMapper.class)
+ .register(JacksonFeature.class);
+ }
+
+ /** Verifies that an uncaught error is converted to a Lance internal error
response. */
+ @Test
+ public void testErrorResponse() {
+ try (Response response =
target("error").request(MediaType.APPLICATION_JSON_TYPE).get()) {
+ Assertions.assertEquals(
+ Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
response.getStatus());
+ ErrorResponse entity = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals("assertion failure", entity.getError());
+ Assertions.assertEquals("", entity.getInstance());
+ Assertions.assertTrue(
+ entity.getDetail().contains("java.lang.AssertionError: assertion
failure"));
+ Assertions.assertTrue(
+ entity.getDetail().contains("Caused by:
java.lang.IllegalStateException: root cause"));
+ }
+ }
+}
diff --git
a/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
b/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
index f2ef1e77a8..3db96fed27 100644
--- a/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
+++ b/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
@@ -62,6 +62,7 @@ import org.apache.gravitino.server.web.RequestContextFilter;
import org.apache.gravitino.server.web.VersioningFilter;
import org.apache.gravitino.server.web.filter.AccessControlNotAllowedFilter;
import org.apache.gravitino.server.web.filter.GravitinoInterceptionService;
+import org.apache.gravitino.server.web.mapper.ErrorExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonMappingExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonParseExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonProcessingExceptionMapper;
@@ -181,6 +182,7 @@ public class GravitinoServer extends ResourceConfig {
}
});
register(JsonProcessingExceptionMapper.class);
+ register(ErrorExceptionMapper.class);
register(JsonParseExceptionMapper.class);
register(JsonMappingExceptionMapper.class);
register(ParamExceptionMapper.class);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/mapper/ErrorExceptionMapper.java
b/server/src/main/java/org/apache/gravitino/server/web/mapper/ErrorExceptionMapper.java
new file mode 100644
index 0000000000..892cc8f699
--- /dev/null
+++
b/server/src/main/java/org/apache/gravitino/server/web/mapper/ErrorExceptionMapper.java
@@ -0,0 +1,47 @@
+/*
+ * 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.server.web.mapper;
+
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.ExceptionMapper;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Reports errors on the request path as server errors without deciding
process lifetime. */
+public class ErrorExceptionMapper implements ExceptionMapper<Error> {
+ private static final Logger LOG =
LoggerFactory.getLogger(ErrorExceptionMapper.class);
+
+ /**
+ * Returns a server error response retaining the original error type and
complete stack trace.
+ *
+ * @param error The error raised while processing the request.
+ * @return The internal server error response.
+ */
+ @Override
+ public Response toResponse(Error error) {
+ String message = "Server error while processing request: " + error;
+ LOG.error(message, error);
+ return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(ErrorResponse.internalError(error.getClass().getSimpleName(),
message, error))
+ .type(MediaType.APPLICATION_JSON_TYPE)
+ .build();
+ }
+}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/mapper/TestErrorExceptionMapper.java
b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestErrorExceptionMapper.java
new file mode 100644
index 0000000000..78d264db46
--- /dev/null
+++
b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestErrorExceptionMapper.java
@@ -0,0 +1,52 @@
+/*
+ * 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.server.web.mapper;
+
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests server error responses for errors raised on the request path. */
+public class TestErrorExceptionMapper {
+ /** Checks that error types and nested causes survive response construction.
*/
+ @Test
+ public void testErrorsRetainTypeAndCause() {
+ for (Error error :
+ new Error[] {
+ new OutOfMemoryError("Metaspace"), new StackOverflowError(),
+ new NoClassDefFoundError("catalog class"), new
AssertionError("assertion")
+ }) {
+ error.initCause(new IllegalStateException("root cause"));
+ try (Response response = new ErrorExceptionMapper().toResponse(error)) {
+ Assertions.assertEquals(500, response.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
response.getMediaType());
+ ErrorResponse entity = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(error.getClass().getSimpleName(),
entity.getType());
+ Assertions.assertEquals(
+ "Server error while processing request: " + error,
entity.getMessage());
+ String stack = String.join("\n", entity.getStack());
+ Assertions.assertTrue(stack.contains(error.toString()));
+ Assertions.assertTrue(
+ stack.contains("Caused by: java.lang.IllegalStateException: root
cause"));
+ }
+ }
+ }
+}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
index 343a778a77..770274b946 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
@@ -70,6 +70,7 @@ import org.apache.gravitino.file.Fileset;
import org.apache.gravitino.file.FilesetChange;
import org.apache.gravitino.lock.LockManager;
import org.apache.gravitino.rest.RESTUtils;
+import org.apache.gravitino.server.web.mapper.ErrorExceptionMapper;
import org.glassfish.jersey.internal.inject.AbstractBinder;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.TestProperties;
@@ -121,6 +122,7 @@ public class TestFilesetOperations extends
BaseOperationsTest {
ResourceConfig resourceConfig = new ResourceConfig();
resourceConfig.register(FilesetOperations.class);
+ resourceConfig.register(ErrorExceptionMapper.class);
resourceConfig.register(
new AbstractBinder() {
@Override
@@ -392,20 +394,42 @@ public class TestFilesetOperations extends
BaseOperationsTest {
Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResp3.getCode());
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResp3.getType());
- // Test throw Error
- doThrow(new Error("mock error"))
+ // A request error must retain its diagnostics without becoming an
operation failure.
+ Error error = new NoClassDefFoundError("mock catalog class");
+ error.initCause(new ClassNotFoundException("missing catalog dependency"));
+ Mockito.doThrow(error)
+ .doReturn(fileset)
.when(dispatcher)
.createMultipleLocationFileset(any(), any(), any(), any(), any());
- Response resp4 =
+ try (Response errorResponse =
target(filesetPath(metalake, catalog, schema))
.request(MediaType.APPLICATION_JSON_TYPE)
.accept("application/vnd.gravitino.v1+json")
- .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE));
- Assertions.assertEquals(
- Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp4.getStatus());
- ErrorResponse errorResp4 = resp4.readEntity(ErrorResponse.class);
- Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResp4.getCode());
- Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResp4.getType());
+ .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(500, errorResponse.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
errorResponse.getMediaType());
+ ErrorResponse entity = errorResponse.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
entity.getCode());
+ Assertions.assertEquals("NoClassDefFoundError", entity.getType());
+ Assertions.assertEquals(
+ "Server error while processing request:
java.lang.NoClassDefFoundError: mock catalog class",
+ entity.getMessage());
+ String stack = String.join("\n", entity.getStack());
+ Assertions.assertTrue(stack.contains("java.lang.NoClassDefFoundError:
mock catalog class"));
+ Assertions.assertTrue(
+ stack.contains(
+ "Caused by: java.lang.ClassNotFoundException: missing catalog
dependency"));
+ }
+
+ try (Response recoveredResponse =
+ target(filesetPath(metalake, catalog, schema))
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(200, recoveredResponse.getStatus());
+ Assertions.assertEquals(
+ "fileset1",
recoveredResponse.readEntity(FilesetResponse.class).getFileset().name());
+ }
}
@Test
