This is an automated email from the ASF dual-hosted git repository.
dimas 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 5c38780e Handle more cloud errors (#864)
5c38780e is described below
commit 5c38780e6130dd5c18346437bc2a6a27e41e9bf6
Author: Andrew Guterman <[email protected]>
AuthorDate: Mon Jan 27 19:55:14 2025 -0800
Handle more cloud errors (#864)
* Forward cloud exception status codes
* Update mappings
* Add 504 => 500 test
* 3xx => 422, 408 => 408, 504 => 504, 5xx => 502, cleanup tests
---
.../src/main/kotlin/polaris-quarkus.gradle.kts | 1 -
.../quarkus/IcebergExceptionMapperTest.java | 62 +++++++++--
service/common/build.gradle.kts | 2 +
.../service/exception/IcebergExceptionMapper.java | 117 +++++++++++++++------
4 files changed, 138 insertions(+), 44 deletions(-)
diff --git a/build-logic/src/main/kotlin/polaris-quarkus.gradle.kts
b/build-logic/src/main/kotlin/polaris-quarkus.gradle.kts
index 20757350..21abf364 100644
--- a/build-logic/src/main/kotlin/polaris-quarkus.gradle.kts
+++ b/build-logic/src/main/kotlin/polaris-quarkus.gradle.kts
@@ -17,7 +17,6 @@
* under the License.
*/
-import gradle.kotlin.dsl.accessors._fa00c0b20184971a79f32516372275b9.testing
import org.gradle.api.attributes.TestSuiteType
import org.gradle.api.plugins.jvm.JvmTestSuite
import org.gradle.kotlin.dsl.register
diff --git
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
index 2b5a2661..ee3ec3e3 100644
---
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
+++
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/IcebergExceptionMapperTest.java
@@ -16,13 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.service.dropwizard;
+package org.apache.polaris.service.quarkus;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import com.azure.core.exception.AzureException;
+import com.azure.core.exception.HttpResponseException;
+import com.azure.core.http.HttpResponse;
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;
@@ -33,14 +38,40 @@ import software.amazon.awssdk.services.s3.model.S3Exception;
class IcebergExceptionMapperTest {
static Stream<Arguments> fileIOExceptionMapping() {
- return Stream.of(
- Arguments.of(new AzureException("Unknown"), 500),
- Arguments.of(new AzureException("Forbidden"), 403),
- Arguments.of(new AzureException("FORBIDDEN"), 403),
- Arguments.of(new AzureException("Not Authorized"), 403),
- Arguments.of(new AzureException("Access Denied"), 403),
- Arguments.of(S3Exception.builder().message("Access denied").build(),
403),
- Arguments.of(new StorageException(1, "access denied"), 403));
+ Map<Integer, Integer> cloudCodeMappings =
+ Map.of(
+ // Map of HTTP code returned from a cloud provider to the HTTP
code Polaris is expected
+ // to return
+ 302, 422,
+ 400, 400,
+ 401, 403,
+ 403, 403,
+ 404, 400,
+ 408, 408,
+ 429, 429,
+ 503, 502,
+ 504, 504);
+
+ return Stream.concat(
+ Stream.of(
+ Arguments.of(new AzureException("Unknown"), 500),
+ Arguments.of(new AzureException("Forbidden"), 403),
+ Arguments.of(new AzureException("FORBIDDEN"), 403),
+ Arguments.of(new AzureException("Not Authorized"), 403),
+ Arguments.of(new AzureException("Access Denied"), 403),
+ Arguments.of(S3Exception.builder().message("Access
denied").build(), 403),
+ Arguments.of(new StorageException(1, "access denied"), 403)),
+ cloudCodeMappings.entrySet().stream()
+ .flatMap(
+ entry ->
+ Stream.of(
+ Arguments.of(
+ new HttpResponseException("",
mockAzureResponse(entry.getKey()), ""),
+ entry.getValue()),
+ Arguments.of(
+
S3Exception.builder().message("").statusCode(entry.getKey()).build(),
+ entry.getValue()),
+ Arguments.of(new StorageException(entry.getKey(), ""),
entry.getValue()))));
}
@ParameterizedTest
@@ -52,4 +83,17 @@ class IcebergExceptionMapperTest {
assertThat(response.getEntity()).extracting("message").isEqualTo(ex.getMessage());
}
}
+
+ /**
+ * Creates a mock of the Azure-specific HttpResponse object, as it's quite
difficult to construct
+ * a "real" one.
+ *
+ * @param statusCode
+ * @return
+ */
+ private static HttpResponse mockAzureResponse(int statusCode) {
+ HttpResponse res = mock(HttpResponse.class);
+ when(res.getStatusCode()).thenReturn(statusCode);
+ return res;
+ }
}
diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts
index a44e8e13..79d2809c 100644
--- a/service/common/build.gradle.kts
+++ b/service/common/build.gradle.kts
@@ -85,6 +85,8 @@ dependencies {
implementation(platform(libs.azuresdk.bom))
implementation("com.azure:azure-core")
+ implementation("com.azure:azure-storage-blob")
+ implementation("com.azure:azure-storage-file-datalake")
testImplementation(platform(libs.junit.bom))
testImplementation("org.junit.jupiter:junit-jupiter")
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 6a210783..0504dd17 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
@@ -19,12 +19,14 @@
package org.apache.polaris.service.exception;
import com.azure.core.exception.AzureException;
+import com.azure.core.exception.HttpResponseException;
import com.google.cloud.storage.StorageException;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
+import jakarta.ws.rs.core.Response.Status;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import java.util.Arrays;
@@ -73,40 +75,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
@Override
public Response toResponse(RuntimeException runtimeException) {
LOGGER.info("Handling runtimeException {}", runtimeException.getMessage());
- int responseCode =
- switch (runtimeException) {
- case NoSuchNamespaceException e ->
Response.Status.NOT_FOUND.getStatusCode();
- case NoSuchIcebergTableException e ->
Response.Status.NOT_FOUND.getStatusCode();
- case NoSuchTableException e ->
Response.Status.NOT_FOUND.getStatusCode();
- case NoSuchViewException e ->
Response.Status.NOT_FOUND.getStatusCode();
- case NotFoundException e ->
Response.Status.NOT_FOUND.getStatusCode();
- case AlreadyExistsException e ->
Response.Status.CONFLICT.getStatusCode();
- case CommitFailedException e ->
Response.Status.CONFLICT.getStatusCode();
- case UnprocessableEntityException e -> 422;
- case CherrypickAncestorCommitException e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case CommitStateUnknownException e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case DuplicateWAPCommitException e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case ForbiddenException e ->
Response.Status.FORBIDDEN.getStatusCode();
- case jakarta.ws.rs.ForbiddenException e ->
Response.Status.FORBIDDEN.getStatusCode();
- case NotAuthorizedException e ->
Response.Status.UNAUTHORIZED.getStatusCode();
- case NamespaceNotEmptyException e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case ValidationException e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case ServiceUnavailableException e ->
Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
- case RuntimeIOException e ->
Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
- case ServiceFailureException e ->
Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
- case CleanableFailure e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case RESTException e ->
Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
- case IllegalArgumentException e ->
Response.Status.BAD_REQUEST.getStatusCode();
- case UnsupportedOperationException e ->
Response.Status.NOT_ACCEPTABLE.getStatusCode();
- case S3Exception e when doesAnyThrowableContainAccessDeniedHint(e) ->
- Response.Status.FORBIDDEN.getStatusCode();
- case AzureException e when
doesAnyThrowableContainAccessDeniedHint(e) ->
- Response.Status.FORBIDDEN.getStatusCode();
- case StorageException e when
doesAnyThrowableContainAccessDeniedHint(e) ->
- Response.Status.FORBIDDEN.getStatusCode();
- case WebApplicationException e -> e.getResponse().getStatus();
- default -> Response.Status.INTERNAL_SERVER_ERROR.getStatusCode();
- };
+ int responseCode = mapExceptionToResponseCode(runtimeException);
if (responseCode == Response.Status.INTERNAL_SERVER_ERROR.getStatusCode())
{
LOGGER.error("Unhandled exception returning INTERNAL_SERVER_ERROR",
runtimeException);
}
@@ -144,4 +113,84 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
public static Collection<String> getAccessDeniedHints() {
return ImmutableSet.copyOf(ACCESS_DENIED_HINTS);
}
+
+ static int mapExceptionToResponseCode(RuntimeException rex) {
+ // Cloud exceptions
+ if (rex instanceof S3Exception
+ || rex instanceof AzureException
+ || rex instanceof StorageException) {
+ return mapCloudExceptionToResponseCode(rex);
+ }
+
+ // Non-cloud exceptions
+ return switch (rex) {
+ case NoSuchNamespaceException e -> Status.NOT_FOUND.getStatusCode();
+ case NoSuchIcebergTableException e -> Status.NOT_FOUND.getStatusCode();
+ case NoSuchTableException e -> Status.NOT_FOUND.getStatusCode();
+ case NoSuchViewException e -> Status.NOT_FOUND.getStatusCode();
+ case NotFoundException e -> Status.NOT_FOUND.getStatusCode();
+ case AlreadyExistsException e -> Status.CONFLICT.getStatusCode();
+ case CommitFailedException e -> Status.CONFLICT.getStatusCode();
+ case UnprocessableEntityException e -> 422;
+ case CherrypickAncestorCommitException e ->
Status.BAD_REQUEST.getStatusCode();
+ case CommitStateUnknownException e -> Status.BAD_REQUEST.getStatusCode();
+ case DuplicateWAPCommitException e -> Status.BAD_REQUEST.getStatusCode();
+ case ForbiddenException e -> Status.FORBIDDEN.getStatusCode();
+ case jakarta.ws.rs.ForbiddenException e ->
Status.FORBIDDEN.getStatusCode();
+ case NotAuthorizedException e -> Status.UNAUTHORIZED.getStatusCode();
+ case NamespaceNotEmptyException e -> Status.BAD_REQUEST.getStatusCode();
+ case ValidationException e -> Status.BAD_REQUEST.getStatusCode();
+ case ServiceUnavailableException e ->
Status.SERVICE_UNAVAILABLE.getStatusCode();
+ case RuntimeIOException e -> Status.SERVICE_UNAVAILABLE.getStatusCode();
+ case ServiceFailureException e ->
Status.SERVICE_UNAVAILABLE.getStatusCode();
+ case CleanableFailure e -> Status.BAD_REQUEST.getStatusCode();
+ case RESTException e -> Status.SERVICE_UNAVAILABLE.getStatusCode();
+ case IllegalArgumentException e -> Status.BAD_REQUEST.getStatusCode();
+ case UnsupportedOperationException e ->
Status.NOT_ACCEPTABLE.getStatusCode();
+ case WebApplicationException e -> e.getResponse().getStatus();
+ default -> Status.INTERNAL_SERVER_ERROR.getStatusCode();
+ };
+ }
+
+ static int mapCloudExceptionToResponseCode(RuntimeException rex) {
+ if (doesAnyThrowableContainAccessDeniedHint(rex)) {
+ return Status.FORBIDDEN.getStatusCode();
+ }
+
+ int httpCode =
+ switch (rex) {
+ case S3Exception s3e -> s3e.statusCode();
+ case HttpResponseException hre -> hre.getResponse().getStatusCode();
+ case StorageException se -> se.getCode();
+ default -> -1;
+ };
+ Status httpStatus = Status.fromStatusCode(httpCode);
+ Status.Family httpFamily = Status.Family.familyOf(httpCode);
+
+ if (httpStatus == Status.NOT_FOUND) {
+ return Status.BAD_REQUEST.getStatusCode();
+ }
+ if (httpStatus == Status.UNAUTHORIZED) {
+ return Status.FORBIDDEN.getStatusCode();
+ }
+ if (httpStatus == Status.BAD_REQUEST
+ || httpStatus == Status.FORBIDDEN
+ || httpStatus == Status.REQUEST_TIMEOUT
+ || httpStatus == Status.TOO_MANY_REQUESTS
+ || httpStatus == Status.GATEWAY_TIMEOUT) {
+ return httpCode;
+ }
+ if (httpFamily == Status.Family.REDIRECTION) {
+ // Currently Polaris doesn't know how to follow redirects from cloud
providers, thus clients
+ // shouldn't expect it to.
+ // This is a 4xx error to indicate that the client may be able to
resolve this by changing
+ // some data, such as their catalog's region.
+ return 422;
+ }
+ if (httpFamily == Status.Family.SERVER_ERROR) {
+ return Status.BAD_GATEWAY.getStatusCode();
+ }
+
+ return Status.INTERNAL_SERVER_ERROR.getStatusCode();
+ }
}
