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 3816cfda Try extracting cloud exceptions from the root cause (#1105)
3816cfda is described below
commit 3816cfda02cf145a1ef35a2c4990203137098e26
Author: Andrew Guterman <[email protected]>
AuthorDate: Tue Mar 4 10:14:29 2025 -0800
Try extracting cloud exceptions from the root cause (#1105)
* Map cloud exceptions using root cause
* Inspect full exception chain, use better variable names for Throwables
* Update comments
---
.../service/exception/IcebergExceptionMapper.java | 54 ++++++++++++----------
.../exception/IcebergExceptionMapperTest.java | 36 +++++++++++----
2 files changed, 56 insertions(+), 34 deletions(-)
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 25e33467..c606194c 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
@@ -33,6 +33,7 @@ import jakarta.ws.rs.ext.Provider;
import java.util.Arrays;
import java.util.Collection;
import java.util.Locale;
+import java.util.Optional;
import java.util.Set;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.iceberg.exceptions.AlreadyExistsException;
@@ -116,12 +117,11 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
}
/**
- * @return whether any throwable in the exception chain
case-insensitive-contains the given
- * message
+ * @return whether any throwable in the chain case-insensitive-contains the
given message
*/
- static boolean doesAnyThrowableContainAccessDeniedHint(Exception e) {
- return Arrays.stream(ExceptionUtils.getThrowables(e))
- .anyMatch(t -> containsAnyAccessDeniedHint(t.getMessage()));
+ static boolean doesAnyThrowableContainAccessDeniedHint(Throwable t) {
+ return Arrays.stream(ExceptionUtils.getThrowables(t))
+ .anyMatch(th -> containsAnyAccessDeniedHint(th.getMessage()));
}
public static boolean containsAnyAccessDeniedHint(String message) {
@@ -130,21 +130,21 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
}
/**
- * Check if the exception is retryable for the storage provider
+ * Check if the Throwable is retryable for the storage provider
*
- * @param ex exception
- * @return true if the exception is retryable
+ * @param t the Throwable
+ * @return true if the Throwable is retryable
*/
- public static boolean isStorageProviderRetryableException(Throwable ex) {
- if (ex == null) {
+ public static boolean isStorageProviderRetryableException(Throwable t) {
+ if (t == null) {
return false;
}
- if (ex.getMessage() != null &&
containsAnyAccessDeniedHint(ex.getMessage())) {
+ if (t.getMessage() != null && containsAnyAccessDeniedHint(t.getMessage()))
{
return false;
}
- return switch (ex) {
+ return switch (t) {
// GCS
case BaseServiceException bse -> bse.isRetryable();
@@ -165,14 +165,14 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
}
static int mapExceptionToResponseCode(RuntimeException rex) {
- // Cloud exceptions
- if (rex instanceof S3Exception
- || rex instanceof AzureException
- || rex instanceof StorageException) {
- return mapCloudExceptionToResponseCode(rex);
+ Optional<Throwable> cloudException =
+ Arrays.stream(ExceptionUtils.getThrowables(rex))
+ .filter(IcebergExceptionMapper::isCloudException)
+ .findAny();
+ if (cloudException.isPresent()) {
+ return mapCloudExceptionToResponseCode(cloudException.get());
}
- // Non-cloud exceptions
return switch (rex) {
case NoSuchNamespaceException e -> Status.NOT_FOUND.getStatusCode();
case NoSuchIcebergTableException e -> Status.NOT_FOUND.getStatusCode();
@@ -202,16 +202,20 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
};
}
+ private static boolean isCloudException(Throwable t) {
+ return t instanceof S3Exception || t instanceof AzureException || t
instanceof StorageException;
+ }
+
/**
* We typically call cloud providers over HTTP, so when there's an exception
there's typically an
* associated HTTP code. This extracts the HTTP code if possible.
*
- * @param rex The cloud provider exception
- * @return UNKNOWN_CLOUD_HTTP_CODE if the exception is not a cloud exception
that we know how to
+ * @param t The cloud provider throwable
+ * @return UNKNOWN_CLOUD_HTTP_CODE if the throwable is not a cloud exception
that we know how to
* extract the code from
*/
- public static int extractHttpCodeFromCloudException(RuntimeException rex) {
- return switch (rex) {
+ public static int extractHttpCodeFromCloudException(Throwable t) {
+ return switch (t) {
case S3Exception s3e -> s3e.statusCode();
case HttpResponseException hre -> hre.getResponse().getStatusCode();
case StorageException se -> se.getCode();
@@ -219,12 +223,12 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
};
}
- static int mapCloudExceptionToResponseCode(RuntimeException rex) {
- if (doesAnyThrowableContainAccessDeniedHint(rex)) {
+ static int mapCloudExceptionToResponseCode(Throwable t) {
+ if (doesAnyThrowableContainAccessDeniedHint(t)) {
return Status.FORBIDDEN.getStatusCode();
}
- int httpCode = extractHttpCodeFromCloudException(rex);
+ int httpCode = extractHttpCodeFromCloudException(t);
Status httpStatus = Status.fromStatusCode(httpCode);
Status.Family httpFamily = Status.Family.familyOf(httpCode);
diff --git
a/service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
b/service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
index 3642234c..7cd5737a 100644
---
a/service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
+++
b/service/common/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
@@ -24,8 +24,10 @@ import com.azure.core.exception.AzureException;
import com.azure.core.exception.HttpResponseException;
import com.google.cloud.storage.StorageException;
import jakarta.ws.rs.core.Response;
+import java.io.IOException;
import java.util.Map;
import java.util.stream.Stream;
+import org.apache.iceberg.exceptions.RuntimeIOException;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
@@ -37,7 +39,11 @@ public class IcebergExceptionMapperTest {
Map<Integer, Integer> cloudCodeMappings =
Map.of(
// Map of HTTP code returned from a cloud provider to the HTTP
code Polaris is expected
- // to return
+ // to return. We create a test case for each of these mappings for
each cloud provider.
+ // We also create a test case for cloud provider exceptions
wrapped as
+ // RuntimeIOExceptions,
+ // which is what the Iceberg SDK sometimes wraps them with if the
error happens during
+ // IO.
302, 422,
400, 400,
401, 403,
@@ -61,14 +67,26 @@ public class IcebergExceptionMapperTest {
.flatMap(
entry ->
Stream.of(
- Arguments.of(
- new HttpResponseException(
- "", new FakeAzureHttpResponse(entry.getKey()),
""),
- entry.getValue()),
- Arguments.of(
-
S3Exception.builder().message("").statusCode(entry.getKey()).build(),
- entry.getValue()),
- Arguments.of(new StorageException(entry.getKey(), ""),
entry.getValue()))));
+ Arguments.of(
+ new HttpResponseException(
+ "", new
FakeAzureHttpResponse(entry.getKey()), ""),
+ entry.getValue()),
+ Arguments.of(
+ S3Exception.builder()
+ .message("")
+ .statusCode(entry.getKey())
+ .build(),
+ entry.getValue()),
+ Arguments.of(
+ new StorageException(entry.getKey(), ""),
entry.getValue()))
+ .flatMap(
+ args ->
+ Stream.of(
+ args,
+ Arguments.of(
+ new RuntimeIOException(
+ new IOException((Throwable)
args.get()[0])),
+ args.get()[1])))));
}
@ParameterizedTest
