This is an automated email from the ASF dual-hosted git repository.
emaynard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new f5b4d6f4 Add a debug log with the full exception (#1023)
f5b4d6f4 is described below
commit f5b4d6f4c59bf5005b044d0e64a30745d84e3b92
Author: Andrew Guterman <[email protected]>
AuthorDate: Mon Feb 24 09:18:16 2025 -0800
Add a debug log with the full exception (#1023)
* clean up
* spotless
* clean up
* refactor message
* no CVEM
* change log levels
* revert polaris-java dep change
* add testImplementation
* uppercase
---
service/common/build.gradle.kts | 2 +
.../service/exception/IcebergExceptionMapper.java | 6 ++
.../IcebergJsonProcessingExceptionMapper.java | 1 +
.../service/exception/PolarisExceptionMapper.java | 10 +++
.../service/exception/ExceptionMapperTest.java | 99 ++++++++++++++++++++++
.../exception}/IcebergExceptionMapperTest.java | 5 +-
6 files changed, 120 insertions(+), 3 deletions(-)
diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts
index 58c99286..3d6c90f4 100644
--- a/service/common/build.gradle.kts
+++ b/service/common/build.gradle.kts
@@ -95,6 +95,8 @@ dependencies {
testImplementation(libs.mockito.core)
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
+ testImplementation(libs.logback.classic)
+
testFixturesImplementation(project(":polaris-core"))
testFixturesImplementation(project(":polaris-api-management-model"))
testFixturesImplementation(project(":polaris-api-management-service"))
diff --git
a/service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
b/service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
index 0504dd17..248aa7f5 100644
---
a/service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
+++
b/service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
@@ -57,6 +57,7 @@ import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.rest.responses.ErrorResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
import software.amazon.awssdk.services.s3.model.S3Exception;
@Provider
@@ -75,7 +76,12 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
@Override
public Response toResponse(RuntimeException runtimeException) {
LOGGER.info("Handling runtimeException {}", runtimeException.getMessage());
+
int responseCode = mapExceptionToResponseCode(runtimeException);
+ LOGGER
+ .atLevel(responseCode >= 500 ? Level.INFO : Level.DEBUG)
+ .log("Full RuntimeException", runtimeException);
+
if (responseCode == Response.Status.INTERNAL_SERVER_ERROR.getStatusCode())
{
LOGGER.error("Unhandled exception returning INTERNAL_SERVER_ERROR",
runtimeException);
}
diff --git
a/service/common/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
b/service/common/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
index 8fc1d7e7..57dcba32 100644
---
a/service/common/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
+++
b/service/common/src/main/java/org/apache/polaris/service/exception/IcebergJsonProcessingExceptionMapper.java
@@ -76,6 +76,7 @@ public final class IcebergJsonProcessingExceptionMapper
* Otherwise, it's those pesky users.
*/
LOGGER.info("Unable to process JSON: {}", exception.getMessage());
+ LOGGER.debug("Full JsonProcessingException", exception);
String messagePrefix =
switch (exception) {
diff --git
a/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
b/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
index 2ba2b804..7686f9f7 100644
---
a/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
+++
b/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
@@ -26,6 +26,9 @@ import org.apache.iceberg.rest.responses.ErrorResponse;
import org.apache.polaris.core.exceptions.AlreadyExistsException;
import org.apache.polaris.core.exceptions.PolarisException;
import org.apache.polaris.service.context.UnresolvableRealmContextException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
/**
* An {@link ExceptionMapper} implementation for {@link PolarisException}s
modeled after {@link
@@ -34,6 +37,8 @@ import
org.apache.polaris.service.context.UnresolvableRealmContextException;
@Provider
public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException> {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PolarisExceptionMapper.class);
+
private Response.Status getStatus(PolarisException exception) {
if (exception instanceof AlreadyExistsException) {
return Response.Status.CONFLICT;
@@ -47,6 +52,11 @@ public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException>
@Override
public Response toResponse(PolarisException exception) {
Response.Status status = getStatus(exception);
+ LOGGER
+ .atLevel(
+ status.getFamily() == Response.Status.Family.SERVER_ERROR ?
Level.INFO : Level.DEBUG)
+ .log("Full PolarisException", exception);
+
ErrorResponse errorResponse =
ErrorResponse.builder()
.responseCode(status.getStatusCode())
diff --git
a/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
b/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
new file mode 100644
index 00000000..2a27f2f3
--- /dev/null
+++
b/service/common/src/test/java/org/apache/polaris/service/exception/ExceptionMapperTest.java
@@ -0,0 +1,99 @@
+/*
+ * 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.polaris.service.exception;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.classic.spi.IThrowableProxy;
+import ch.qos.logback.core.read.ListAppender;
+import com.fasterxml.jackson.core.JsonLocation;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import jakarta.ws.rs.ext.ExceptionMapper;
+import java.util.Optional;
+import java.util.stream.Stream;
+import org.apache.polaris.core.exceptions.AlreadyExistsException;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.LoggerFactory;
+
+/** Unit tests for exception mappers */
+public class ExceptionMapperTest {
+ private static final String MESSAGE = "this is the exception message";
+ private static final String CAUSE = "this is the exception cause";
+
+ @ParameterizedTest
+ @MethodSource("testFullExceptionIsLogged")
+ public void testFullExceptionIsLogged(ExceptionMapper mapper, Exception
exception, Level level) {
+ Logger logger = (Logger) LoggerFactory.getLogger(mapper.getClass());
+ ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
+ listAppender.start();
+ logger.addAppender(listAppender);
+
+ mapper.toResponse(exception);
+
+ Assertions.assertThat(
+ listAppender.list.stream()
+ .anyMatch(
+ log -> {
+ if (log.getLevel() != level) {
+ return false;
+ }
+
+ IThrowableProxy proxy = log.getThrowableProxy();
+ if (proxy == null) {
+ return false;
+ }
+
+ return proxy.getMessage().contains(CAUSE)
+ || Optional.ofNullable(proxy.getCause())
+ .map(IThrowableProxy::getMessage)
+ .orElse("")
+ .contains(CAUSE);
+ }))
+ .as("The exception cause should be logged")
+ .isTrue();
+ }
+
+ static Stream<Arguments> testFullExceptionIsLogged() {
+ // ConstraintViolationException isn't included because it doesn't
propagate any info to its
+ // inherited Exception
+ return Stream.of(
+ Arguments.of(
+ new IcebergExceptionMapper(),
+ new RuntimeException(MESSAGE, new RuntimeException(CAUSE)),
+ Level.ERROR),
+ Arguments.of(
+ new IcebergJsonProcessingExceptionMapper(),
+ new TestJsonProcessingException(MESSAGE, null, new
RuntimeException(CAUSE)),
+ Level.DEBUG),
+ Arguments.of(
+ new PolarisExceptionMapper(),
+ new AlreadyExistsException(MESSAGE, new RuntimeException(CAUSE)),
+ Level.DEBUG));
+ }
+
+ private static class TestJsonProcessingException extends
JsonProcessingException {
+ protected TestJsonProcessingException(String msg, JsonLocation loc,
Throwable rootCause) {
+ super(msg, loc, rootCause);
+ }
+ }
+}
diff --git
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
b/service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
similarity index 96%
rename from
quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
rename to
service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
index ee3ec3e3..e991b79d 100644
---
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
+++
b/service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service.quarkus;
+package org.apache.polaris.service.exception;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
@@ -29,13 +29,12 @@ import com.google.cloud.storage.StorageException;
import jakarta.ws.rs.core.Response;
import java.util.Map;
import java.util.stream.Stream;
-import org.apache.polaris.service.exception.IcebergExceptionMapper;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import software.amazon.awssdk.services.s3.model.S3Exception;
-class IcebergExceptionMapperTest {
+public class IcebergExceptionMapperTest {
static Stream<Arguments> fileIOExceptionMapping() {
Map<Integer, Integer> cloudCodeMappings =
