andrew4699 commented on code in PR #1159:
URL: https://github.com/apache/polaris/pull/1159#discussion_r1990019996
##########
service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -165,12 +167,16 @@ public static Collection<String> getAccessDeniedHints() {
}
static int mapExceptionToResponseCode(RuntimeException rex) {
- Optional<Throwable> cloudException =
- Arrays.stream(ExceptionUtils.getThrowables(rex))
- .filter(IcebergExceptionMapper::isCloudException)
- .findAny();
- if (cloudException.isPresent()) {
- return mapCloudExceptionToResponseCode(cloudException.get());
+ for (Throwable t : ExceptionUtils.getThrowables(rex)) {
+ // Cloud exceptions can be wrapped by the Iceberg SDK
+ if (isCloudException(t)) {
+ return mapCloudExceptionToResponseCode(t);
+ }
+
+ // UnknownHostException isn't a RuntimeException so it's always wrapped
+ if (t instanceof UnknownHostException &&
t.getMessage().contains(AZURE_STORAGE_URL_SUFFIX)) {
+ return Status.NOT_FOUND.getStatusCode(); // Return a 404 to be
consistent with S3/GCS
+ }
Review Comment:
You're right but the current behavior is to only check the access denied
hints for cloud exception types. Whether that's good or not I guess is its own
discussion. There's some inherent difficulty here by playing a reactive role to
exceptions and not knowing them all ahead of time.
--
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]
