eric-maynard commented on code in PR #1159:
URL: https://github.com/apache/polaris/pull/1159#discussion_r1990047048
##########
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:
`AZURE_STORAGE_URL_SUFFIX` looks as cloud-specific as any exception type --
I think my reaction is mostly to having that relatively low-level detail in the
top-level method `mapExceptionToResponseCode`. If we want to push this into
another method I think that's okay too.
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
