dimas-b commented on code in PR #1159:
URL: https://github.com/apache/polaris/pull/1159#discussion_r1989980744
##########
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:
`UnknownHostException` is not specific to any cloud SDK. I think it's fine
to have it as a special case here.
##########
service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -68,6 +68,8 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
/** Signifies that we could not extract an HTTP code from a given cloud
exception */
public static final int UNKNOWN_CLOUD_HTTP_CODE = -1;
+ @VisibleForTesting public static final String AZURE_STORAGE_URL_SUFFIX =
".blob.core.windows.net";
Review Comment:
nit: This is not the only possible Azure suffix, AFAIK
##########
service/common/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java:
##########
@@ -68,6 +68,8 @@ public class IcebergExceptionMapper implements
ExceptionMapper<RuntimeException>
/** Signifies that we could not extract an HTTP code from a given cloud
exception */
public static final int UNKNOWN_CLOUD_HTTP_CODE = -1;
+ @VisibleForTesting public static final String AZURE_STORAGE_URL_SUFFIX =
".blob.core.windows.net";
Review Comment:
I believe it is preferable to duplicate the string in test code rather that
expose a constant with a narrow purpose. It's easy to adjust tests if/when new
cases are added.
--
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]
