dimas-b commented on code in PR #864:
URL: https://github.com/apache/polaris/pull/864#discussion_r1929222919
##########
service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -144,4 +112,56 @@ public static boolean containsAnyAccessDeniedHint(String
message) {
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) {
+ if (doesAnyThrowableContainAccessDeniedHint(rex)) {
+ return Response.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;
+ };
+
+ if (300 <= httpCode && httpCode <= 499) {
+ return httpCode;
Review Comment:
> 5xx errors should be reserved for incorrect actions the server is taking
with the information it has, rather than correct actions taken with invalid
client-provided information.
This matches 500 fairly well, but not all 5xx errors, IMHO.
That said, what is "client-provided information"? In the context of this
exception mapper, if the client is the REST Catalog, for example, it did not
provide catalog configuration as part of the request it sent.
I suppose it may be necessary to have different exception mappers for the
REST Catalog API and the Management API for fine-tuning error responses. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
