(polaris) branch main updated: fix(rest): return 500 for CommitStateUnknownException per Iceberg REST spec (#5145)

[yu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

flyrain pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git


The following commit(s) were added to refs/heads/main by this push:
     new c4f59da4dd fix(rest): return 500 for CommitStateUnknownException per Iceberg REST spec (#5145)
c4f59da4dd is described below

commit c4f59da4dd1067451e4dcda2c72e89b2cb6944aa
Author: vignesh_A <16...@users.noreply.github.com>
AuthorDate: Fri Jul 31 22:10:47 2026 +0530

    fix(rest): return 500 for CommitStateUnknownException per Iceberg REST spec (#5145)
---
 CHANGELOG.md                                       |  1 +
 .../service/exception/IcebergExceptionMapper.java  |  2 +-
 .../exception/IcebergExceptionMapperTest.java      | 24 ++++++++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6b74e1e061..11f45a13b9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -102,6 +102,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
   API read fails. Previously, individual failures were logged but the command printed incomplete
   configuration and exited with status 0.
 - Python CLI `setup apply` now exits with an error after any setup operation fails, while still attempting the remaining operations. Previously, individual failures were logged but the command reported success and exited with status 0.
+- The Iceberg REST catalog now returns HTTP 500 (instead of 400) when a commit's outcome is unknown (`CommitStateUnknownException`), as required by the Iceberg REST spec. Previously, clients received `BadRequestException` for a commit that may have been applied, which could lead to unsafe retries.
 - Python CLI `tables list`, `tables get`, and `tables delete` commands now exit with status 1 when catalog API requests fail. Previously, these commands printed an error but exited successfully.
 - Default table storage locations with object-storage prefixing enabled are now percent-encoded with UTF-8 instead of the JVM default charset. Previously the persisted location of a table under a non-ASCII namespace or table name depended on the platform default charset, so the same table could resolve to a different (or lossy) location on a non-UTF-8 JVM.
 - Async task execution (table cleanup, manifest and batch file cleanup) now retries when a handler returns false on transient errors (e.g. IO or delete failures). Previously `false` was swallowed with only a warning log and the task was never retried via the existing retry mechanism.
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java b/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
index 105c4abd67..628256d247 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/exception/IcebergExceptionMapper.java
@@ -177,7 +177,7 @@ public class IcebergExceptionMapper implements ExceptionMapper<RuntimeException>
       case CommitFailedException e -> Status.CONFLICT.getStatusCode();
       case UnprocessableEntityException e -> 422;
       case CherrypickAncestorCommitException e -> Status.BAD_REQUEST.getStatusCode();
-      case CommitStateUnknownException e -> Status.BAD_REQUEST.getStatusCode();
+      case CommitStateUnknownException e -> Status.INTERNAL_SERVER_ERROR.getStatusCode();
       case DuplicateWAPCommitException e -> Status.BAD_REQUEST.getStatusCode();
       case ForbiddenException e -> Status.FORBIDDEN.getStatusCode();
       case jakarta.ws.rs.ForbiddenException e -> Status.FORBIDDEN.getStatusCode();
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java b/runtime/service/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
index 4dd0d679d8..48c51109a6 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/exception/IcebergExceptionMapperTest.java
@@ -28,7 +28,11 @@ import java.io.IOException;
 import java.net.UnknownHostException;
 import java.util.Map;
 import java.util.stream.Stream;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.CommitFailedException;
+import org.apache.iceberg.exceptions.CommitStateUnknownException;
 import org.apache.iceberg.exceptions.RuntimeIOException;
+import org.apache.iceberg.exceptions.ValidationException;
 import org.apache.polaris.core.exceptions.FileIOUnknownHostException;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -106,4 +110,24 @@ public class IcebergExceptionMapperTest {
       assertThat(response.getEntity()).extracting("message").isEqualTo(ex.getMessage());
     }
   }
+
+  static Stream<Arguments> coreExceptionMapping() {
+    return Stream.of(
+        // Commit state unknown means the commit may or may not have been applied; per the Iceberg
+        // REST spec it must be a 5xx so clients do not mistake it for a rejected request.
+        Arguments.of(new CommitStateUnknownException(new RuntimeException("db timeout")), 500),
+        Arguments.of(new CommitFailedException("commit failed"), 409),
+        Arguments.of(new AlreadyExistsException("already exists"), 409),
+        Arguments.of(new ValidationException("invalid"), 400));
+  }
+
+  @ParameterizedTest
+  @MethodSource
+  void coreExceptionMapping(RuntimeException ex, int statusCode) {
+    IcebergExceptionMapper mapper = new IcebergExceptionMapper();
+    try (Response response = mapper.toResponse(ex)) {
+      assertThat(response.getStatus()).isEqualTo(statusCode);
+      assertThat(response.getEntity()).extracting("message").isEqualTo(ex.getMessage());
+    }
+  }
 }