From 6ccc893d9bf1b5eac86e4d7f42ee5d6d0d6be4b7 Mon Sep 17 00:00:00 2001
From: Honah J <honahx@apache.org>
Date: Fri, 22 Aug 2025 19:10:58 -0500
Subject: [PATCH] Check return status of CreateCatalogResult, fix NPE

---
 .../service/admin/PolarisAdminService.java    |  7 +++
 .../service/admin/PolarisServiceImpl.java     |  2 +-
 .../service/admin/ManagementServiceTest.java  | 43 +++++++++++++++++++
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
index 9462e13d6a..69f1c1006a 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
@@ -781,6 +781,13 @@ public PolarisEntity createCatalog(CreateCatalogRequest catalogRequest) {
       throw new AlreadyExistsException(
           "Cannot create Catalog %s. Catalog already exists or resolution failed",
           entity.getName());
+    } else if (!catalogResult.isSuccess()) {
+      throw new IllegalStateException(
+          String.format(
+              "Cannot create Catalog %s: %s with extraInfo %s",
+              entity.getName(),
+              catalogResult.getReturnStatus(),
+              catalogResult.getExtraInformation()));
     }
     return PolarisEntity.of(catalogResult.getCatalog());
   }
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java
index 806289da08..fb079cf644 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java
@@ -164,7 +164,7 @@ public Response createCatalog(
     Catalog catalog = request.getCatalog();
     validateStorageConfig(catalog.getStorageConfigInfo());
     validateExternalCatalog(catalog);
-    Catalog newCatalog = new CatalogEntity(adminService.createCatalog(request)).asCatalog();
+    Catalog newCatalog = CatalogEntity.of(adminService.createCatalog(request)).asCatalog();
     LOGGER.info("Created new catalog {}", newCatalog);
     return Response.status(Response.Status.CREATED).build();
   }
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java
index d5a8b9c860..14e086478b 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java
@@ -49,13 +49,16 @@
 import org.apache.polaris.core.entity.PrincipalRoleEntity;
 import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
 import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.core.persistence.dao.entity.BaseResult;
 import org.apache.polaris.core.persistence.dao.entity.CreateCatalogResult;
 import org.apache.polaris.core.persistence.dao.entity.EntityResult;
 import org.apache.polaris.core.secrets.UnsafeInMemorySecretsManager;
 import org.apache.polaris.service.TestServices;
 import org.apache.polaris.service.config.ReservedProperties;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 public class ManagementServiceTest {
   private TestServices services;
@@ -285,4 +288,44 @@ public void testCanListCatalogs() {
         .extracting(Catalog::getName)
         .containsExactlyInAnyOrder("my-catalog-1", "my-catalog-2");
   }
+
+  @Test
+  public void testCreateCatalogReturnErrorOnFailure() {
+    PolarisMetaStoreManager metaStoreManager = Mockito.spy(setupMetaStoreManager());
+    PolarisCallContext callContext = services.newCallContext();
+    PolarisAdminService polarisAdminService =
+        setupPolarisAdminService(metaStoreManager, callContext);
+
+    AwsStorageConfigInfo awsConfigModel =
+        AwsStorageConfigInfo.builder()
+            .setRoleArn("arn:aws:iam::123456789012:role/my-role")
+            .setExternalId("externalId")
+            .setUserArn("userArn")
+            .setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
+            .setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
+            .build();
+    String catalogName = "mycatalog";
+    Catalog catalog =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName)
+            .setProperties(new CatalogProperties("s3://bucket/path/to/data"))
+            .setStorageConfigInfo(awsConfigModel)
+            .build();
+    CreateCatalogResult resultWithError =
+        new CreateCatalogResult(
+            BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unexpected Error Occurred");
+    Mockito.doAnswer(invocation -> resultWithError)
+        .when(metaStoreManager)
+        .createCatalog(Mockito.any(), Mockito.any(), Mockito.any());
+    Assertions.assertThatThrownBy(
+            () -> polarisAdminService.createCatalog(new CreateCatalogRequest(catalog)))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage(
+            String.format(
+                "Cannot create Catalog %s: %s with extraInfo %s",
+                catalogName,
+                resultWithError.getReturnStatus(),
+                resultWithError.getExtraInformation()));
+  }
 }