collado-mike commented on code in PR #864:
URL: https://github.com/apache/polaris/pull/864#discussion_r1930914532


##########
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:
   I agree with all of these except the `502`. I'd really like to make any 
user-configuration problems not a 5xx error code. `5xx` generally cause 
engineers to have to look into the errors and these really ought to be 
user-configuration errors. I think they ought to be `400` errors.



-- 
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]

Reply via email to