ACC-2687 Reproduce bug

contentgrid-appserver-contentstore-api/src/main/java/com/contentgrid/appserver/contentstore/api/ContentStoreException.java

@@ -0,0 +1,17 @@
+package com.contentgrid.appserver.contentstore.api;
+
+/**
+ * Unchecked exception wrapping a {@link ContentIOException} for propagation through
+ * layers that do not declare checked exceptions.
+ */
+public class ContentStoreException extends RuntimeException {
+
+    public ContentStoreException(ContentIOException cause) {
+        super(cause);
+    }
+
+    @Override
+    public ContentIOException getCause() {
+        return (ContentIOException) super.getCause();
+    }
+}

contentgrid-appserver-contentstore-impl-s3/src/main/java/com/contentgrid/appserver/contentstore/impl/s3/S3ContentStore.java

@@ -92,6 +92,7 @@ private static long contentSize(GetObjectResponse response) {
     }
 
     @Override
+    @SneakyThrows(InterruptedException.class)
     public ContentAccessor writeContent(@NonNull InputStream inputStream) throws UnwritableContentException {
         var contentReference = ContentReference.of(UUID.randomUUID().toString());
         try {
@@ -100,24 +101,25 @@ public ContentAccessor writeContent(@NonNull InputStream inputStream) throws Unw
                             .object(contentReference.getValue())
                             .stream(inputStream, -1, PART_SIZE)
                             .build())
-                    .join();
+                    .get();
             return new S3ContentAccessor(contentReference);
         } catch (InsufficientDataException | InternalException | InvalidKeyException | IOException |
-                 NoSuchAlgorithmException | XmlParserException e) {
+                 NoSuchAlgorithmException | XmlParserException | ExecutionException e) {
             throw new UnwritableContentException(contentReference, e);
         }
     }
 
     @Override
+    @SneakyThrows(InterruptedException.class)
     public void remove(@NonNull ContentReference contentReference) throws UnwritableContentException {
         try {
             client.removeObject(RemoveObjectArgs.builder()
                             .bucket(bucketName)
                             .object(contentReference.getValue())
                     .build())
-                    .join();
+                    .get();
         } catch (InsufficientDataException | InternalException | InvalidKeyException | IOException |
-                 NoSuchAlgorithmException | XmlParserException e) {
+                 NoSuchAlgorithmException | XmlParserException | ExecutionException e) {
             throw new UnwritableContentException(contentReference, e);
         }
 

contentgrid-appserver-contentstore-impl-s3/src/test/java/com/contentgrid/appserver/contentstore/impl/s3/S3ContentStoreWriteFailureTest.java

@@ -0,0 +1,125 @@
+package com.contentgrid.appserver.contentstore.impl.s3;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.contentgrid.appserver.contentstore.api.UnwritableContentException;
+import io.minio.MakeBucketArgs;
+import io.minio.MinioAsyncClient;
+import io.minio.RemoveBucketArgs;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.UUID;
+import java.util.concurrent.CompletionException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.testcontainers.containers.MinIOContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
+
+/**
+ * Reproducer for content upload bug where S3 write failures escape as uncaught
+ * {@link CompletionException} instead of being wrapped as {@link UnwritableContentException}.
+ * <p>
+ * When {@link S3ContentStore#writeContent} fails (e.g. broken pipe to S3), the
+ * {@code CompletionException} from {@code CompletableFuture.join()} is not caught.
+ * This causes the exception to propagate uncaught through the entire request processing chain,
+ * bypassing the content store error handling in ContentUploadAttributeMapper and preventing
+ * the database transaction from ever starting. The result is a PUT that appears to succeed
+ * (HTTP 200 with default status) but content metadata is never persisted, causing subsequent
+ * GET requests to return 404.
+ */
+@Testcontainers
+class S3ContentStoreWriteFailureTest {
+
+    @Container
+    private static final MinIOContainer minioContainer = new MinIOContainer("minio/minio:RELEASE.2025-07-23T15-54-02Z");
+
+    private MinioAsyncClient client;
+
+    @BeforeEach
+    void setUp() {
+        client = MinioAsyncClient.builder()
+                .endpoint(minioContainer.getS3URL())
+                .credentials(minioContainer.getUserName(), minioContainer.getPassword())
+                .build();
+    }
+
+    /**
+     * When the S3 bucket does not exist, putObject fails and .join() throws CompletionException.
+     * S3ContentStore.writeContent() should catch this and wrap it as UnwritableContentException,
+     * but currently the CompletionException escapes uncaught.
+     */
+    @Test
+    void writeContent_whenS3OperationFails_shouldThrowUnwritableContentException() {
+        var store = new S3ContentStore(client, "non-existent-bucket-" + UUID.randomUUID());
+
+        assertThrows(UnwritableContentException.class, () -> store.writeContent(
+                new ByteArrayInputStream("test data".getBytes(StandardCharsets.UTF_8))));
+    }
+
+    /**
+     * Simulates a broken pipe during content upload by using an InputStream that fails mid-read.
+     * This mirrors the production scenario where the connection to S3 breaks during upload.
+     * The resulting CompletionException (wrapping an IOException) should be caught and wrapped
+     * as UnwritableContentException.
+     */
+    @Test
+    void writeContent_whenInputStreamFailsDuringUpload_shouldThrowUnwritableContentException() throws Exception {
+        var bucketName = "test-" + UUID.randomUUID();
+        client.makeBucket(MakeBucketArgs.builder().bucket(bucketName).build()).join();
+
+        var store = new S3ContentStore(client, bucketName);
+
+        var brokenInputStream = new InputStream() {
+            private int bytesRead = 0;
+
+            @Override
+            public int read() throws IOException {
+                if (bytesRead++ > 100) {
+                    throw new IOException("Broken pipe");
+                }
+                return 'x';
+            }
+
+            @Override
+            public int read(byte[] b, int off, int len) throws IOException {
+                if (bytesRead > 100) {
+                    throw new IOException("Broken pipe");
+                }
+                int toRead = Math.min(len, 50);
+                for (int i = 0; i < toRead; i++) {
+                    b[off + i] = 'x';
+                }
+                bytesRead += toRead;
+                return toRead;
+            }
+        };
+
+        assertThrows(UnwritableContentException.class, () -> store.writeContent(brokenInputStream));
+    }
+
+    /**
+     * Verifies that remove() has the same problem: CompletionException from .join()
+     * is not caught when the bucket doesn't exist.
+     */
+    @Test
+    void remove_whenS3OperationFails_shouldThrowUnwritableContentException() throws Exception {
+        var bucketName = "test-" + UUID.randomUUID();
+        client.makeBucket(MakeBucketArgs.builder().bucket(bucketName).build()).join();
+
+        var store = new S3ContentStore(client, bucketName);
+
+        // Write content, then delete the bucket to force the remove to fail
+        var accessor = store.writeContent(
+                new ByteArrayInputStream("test".getBytes(StandardCharsets.UTF_8)));
+
+        // Remove the bucket's contents and the bucket itself to cause subsequent operations to fail
+        store.remove(accessor.getReference());
+        client.removeBucket(RemoveBucketArgs.builder().bucket(bucketName).build()).join();
+
+        // Now the store references a non-existent bucket, so remove should fail with UnwritableContentException
+        assertThrows(UnwritableContentException.class, () -> store.remove(accessor.getReference()));
+    }
+}

contentgrid-appserver-domain/build.gradle

@@ -11,7 +11,7 @@ dependencies {
     api 'com.fasterxml.jackson.core:jackson-core'
 
     implementation 'org.slf4j:slf4j-api'
-    implementation project(':contentgrid-appserver-contentstore-api')
+    api project(':contentgrid-appserver-contentstore-api')
     implementation 'org.springframework:spring-core'
     implementation 'com.contentgrid.hateoas:contentgrid-pagination-offset:0.0.4'
 

contentgrid-appserver-domain/src/main/java/com/contentgrid/appserver/domain/data/mapper/ContentUploadAttributeMapper.java

@@ -4,6 +4,7 @@
 import com.contentgrid.appserver.application.model.attributes.ContentAttribute;
 import com.contentgrid.appserver.application.model.attributes.SimpleAttribute;
 import com.contentgrid.appserver.application.model.values.AttributePath;
+import com.contentgrid.appserver.contentstore.api.ContentStoreException;
 import com.contentgrid.appserver.contentstore.api.ContentStore;
 import com.contentgrid.appserver.contentstore.api.UnwritableContentException;
 import com.contentgrid.appserver.domain.data.DataEntry;
@@ -78,8 +79,10 @@ protected Optional<DataEntry> mapCompositeAttributeUnsupportedDatatype(Attribute
                 }
 
                 return Optional.of(builder.build());
-            } catch (UnwritableContentException|IOException e) {
-                throw new RuntimeException(e);
+            } catch (UnwritableContentException e) {
+                throw new ContentStoreException(e);
+            } catch (IOException e) {
+                throw new java.io.UncheckedIOException(e);
             }
 
         }

contentgrid-appserver-rest/src/main/java/com/contentgrid/appserver/rest/problem/ProblemDetailsExceptionHandler.java

@@ -1,5 +1,6 @@
 package com.contentgrid.appserver.rest.problem;
 
+import com.contentgrid.appserver.contentstore.api.ContentStoreException;
 import com.contentgrid.appserver.application.model.Application;
 import com.contentgrid.appserver.domain.data.InvalidDataFormatException;
 import com.contentgrid.appserver.domain.data.InvalidDataTypeException;
@@ -390,6 +391,12 @@ ResponseEntity<Problem> integrity(EntityLinkedByRequiredRelationException except
                 )));
     }
 
+    @ExceptionHandler
+    ResponseEntity<Problem> contentStoreError(ContentStoreException exception) {
+        return createResponse(problemFactory.createProblem(ProblemType.INTERNAL_CONTENT_STORE)
+                .withStatus(HttpStatus.INTERNAL_SERVER_ERROR));
+    }
+
     @ExceptionHandler
     ResponseEntity<?> permissionDenied(PermissionDeniedException exception) {
         return ResponseEntity.status(HttpStatus.FORBIDDEN).build();

contentgrid-appserver-rest/src/main/java/com/contentgrid/appserver/rest/problem/ProblemType.java

@@ -36,6 +36,8 @@ public enum ProblemType implements ProblemTypeResolvable {
 
     INTEGRITY_RELATION_BLIND_OVERWRITE("integrity", "blind-relation-overwrite"),
     INTEGRITY_REQUIRED_RELATION("integrity", "required-relation"),
+
+    INTERNAL_CONTENT_STORE("internal", "content-store"),
     ;
 
     ProblemType(String... params) {

contentgrid-appserver-rest/src/main/resources/com/contentgrid/appserver/rest/problem/messages.properties

@@ -68,3 +68,9 @@ com.contentgrid.appserver.rest.problem.ProblemType.title.integrity.blind-relatio
 com.contentgrid.appserver.rest.problem.ProblemType.detail.integrity.blind-relation-overwrite={1} is already referenced by {2}, it can not be referenced from {0} as well
 com.contentgrid.appserver.rest.problem.ProblemType.title.integrity.required-relation=Relation is required
 com.contentgrid.appserver.rest.problem.ProblemType.detail.integrity.required-relation={0} is required
+
+# Internal
+com.contentgrid.appserver.rest.problem.ProblemType.title.internal=Internal server error
+com.contentgrid.appserver.rest.problem.ProblemType.detail.internal=
+com.contentgrid.appserver.rest.problem.ProblemType.title.internal.content-store=Content store error
+com.contentgrid.appserver.rest.problem.ProblemType.detail.internal.content-store=

contentgrid-appserver-rest/src/test/java/com/contentgrid/appserver/rest/property/ContentRestControllerTest.java

@@ -24,6 +24,8 @@
 import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
 import java.util.UUID;
+import com.contentgrid.appserver.contentstore.api.ContentReference;
+import com.contentgrid.appserver.contentstore.api.UnwritableContentException;
 import java.util.stream.Stream;
 import org.apache.tomcat.util.http.parser.ContentRange;
 import org.junit.jupiter.api.AfterEach;
@@ -954,6 +956,69 @@ void upload_multipart_empty_file_success(HttpMethod method) throws Exception {
                     .andExpect(content().bytes(new byte[0]));
         }
 
+        /**
+         * Reproducer: when the content store write fails with a CompletionException
+         * (e.g. broken pipe to S3), the exception escapes uncaught through S3ContentStore,
+         * ContentUploadAttributeMapper, and the controller. The response should be a server
+         * error (5xx), but because the CompletionException bypasses all catch blocks, the
+         * response status depends on unspecified error handling behavior.
+         *
+         * Additionally, the content must not be persisted in the database — a subsequent GET
+         * should return 404.
+         */
+        @ParameterizedTest
+        @CsvSource({"PUT", "POST"})
+        void upload_contentStoreWriteFails_returnsServerError(HttpMethod method) throws Exception {
+            String invoiceId = createInvoice(null);
+
+            // Simulate S3 broken pipe: S3ContentStore catches the CompletionException from
+            // MinioAsyncClient.putObject().get() and wraps it as UnwritableContentException
+            Mockito.doThrow(new UnwritableContentException(
+                            ContentReference.of("test"), new java.io.IOException("Broken pipe")))
+                    .when(contentStoreSpy).writeContent(Mockito.any());
+
+            mockMvc.perform(request(method, "/invoices/{instanceId}/content", invoiceId)
+                            .contentType(INVOICE_CONTENT_FILE.getContentType())
+                            .content(INVOICE_CONTENT_FILE.getBytes()))
+                    .andExpect(status().is5xxServerError());
+
+            Mockito.reset(contentStoreSpy);
+
+            // Content must not have been persisted
+            mockMvc.perform(get("/invoices/{instanceId}/content", invoiceId))
+                    .andExpect(status().isNotFound());
+        }
+
+        /**
+         * Same as above but with multipart upload, which is the exact path triggered
+         * by the Python client in production.
+         */
+        @Test
+        void upload_multipart_contentStoreWriteFails_returnsServerError() throws Exception {
+            String invoiceId = createInvoice(null);
+
+            Mockito.doThrow(new UnwritableContentException(
+                            ContentReference.of("test"), new java.io.IOException("Broken pipe")))
+                    .when(contentStoreSpy).writeContent(Mockito.any());
+
+            MockMultipartFile file = new MockMultipartFile(
+                    "file",
+                    INVOICE_CONTENT_FILE.getOriginalFilename(),
+                    INVOICE_CONTENT_FILE.getContentType(),
+                    INVOICE_CONTENT_FILE.getBytes()
+            );
+
+            mockMvc.perform(multipart("/invoices/{instanceId}/content", invoiceId)
+                            .file(file))
+                    .andExpect(status().is5xxServerError());
+
+            Mockito.reset(contentStoreSpy);
+
+            // Content must not have been persisted
+            mockMvc.perform(get("/invoices/{instanceId}/content", invoiceId))
+                    .andExpect(status().isNotFound());
+        }
+
         @Test
         void upload_partial_put_not_supported() throws Exception {
             String invoiceId = createInvoice(INVOICE_CONTENT_FILE);