diff --git a/docs/guides/error-handling.md b/docs/guides/error-handling.md index 9390a70..4f2c4d4 100644 --- a/docs/guides/error-handling.md +++ b/docs/guides/error-handling.md @@ -27,6 +27,7 @@ try { | `D1RateLimitException` | Cloudflare returned HTTP 429. | | `D1QueryException` | D1 accepted the request but the SQL operation failed. | | `D1BatchException` | At least one batch item failed. | +| `D1RawBatchException` | At least one raw batch item failed. | | `D1MappingException` | Typed row mapping failed. | | `D1TimeoutException` | Transport timed out. | | `D1TransportException` | Network or transport failure. | @@ -46,3 +47,7 @@ try { ## Sensitive Data Exception messages do not include API tokens or SQL parameter values. `D1QueryException.sql()` may expose SQL text for diagnostics, so handle it carefully when SQL text itself is sensitive. + +`D1BatchException` and `D1RawBatchException` expose the first failed item through +`failedIndex()`. Their `partialResults()` lists are immutable and include all result items returned +by the API, including items after the first failure. diff --git a/docs/guides/raw.md b/docs/guides/raw.md index bfab66a..f4ec51a 100644 --- a/docs/guides/raw.md +++ b/docs/guides/raw.md @@ -34,6 +34,11 @@ List results = d1.rawBatch( Empty raw batches are rejected. +If an item-level result fails, `rawBatch(...)` throws `D1RawBatchException`. Use +`failedIndex()` to locate the first failed item and `partialResults()` to inspect the immutable list +of raw results returned by D1. HTTP failures and top-level API failures continue to use the standard +`D1ApiException` hierarchy. + ## Async Raw Queries `D1AsyncClient` provides matching asynchronous methods: diff --git a/docs/implementation-requirements/02-models-and-errors.md b/docs/implementation-requirements/02-models-and-errors.md index 311301a..fd16b3f 100644 --- a/docs/implementation-requirements/02-models-and-errors.md +++ b/docs/implementation-requirements/02-models-and-errors.md @@ -148,7 +148,8 @@ D1Exception │ ├─ D1AuthorizationException │ ├─ D1RateLimitException │ ├─ D1QueryException - │ └─ D1BatchException + │ ├─ D1BatchException + │ └─ D1RawBatchException ├─ D1MappingException ├─ D1TimeoutException └─ D1TransportException @@ -217,6 +218,23 @@ Rules: - `failedIndex()` returns the first failed result index. - `partialResults()` must be immutable. +## D1RawBatchException + +Required API: + +```java +public int failedIndex(); + +public List partialResults(); +``` + +Rules: + +- `failedIndex()` returns the first failed raw result index. +- `partialResults()` must be immutable. +- Only item-level raw batch failures use `D1RawBatchException`; HTTP and top-level API failures keep + their normal status-based exception mapping. + ## D1MappingException Required API: @@ -245,6 +263,7 @@ Rules: - HTTP 2xx with top-level `success=false`: `D1ApiException` family - Query result `success=false`: `D1QueryException` - Batch result with any failed item: `D1BatchException` +- Raw batch result with any failed item: `D1RawBatchException` - Network failure: `D1TransportException` - Timeout: `D1TimeoutException` - Non-JSON error body: `D1ApiException` with `rawBody()` diff --git a/docs/implementation-requirements/06-internal-architecture.md b/docs/implementation-requirements/06-internal-architecture.md index 8776b12..9d1c309 100644 --- a/docs/implementation-requirements/06-internal-architecture.md +++ b/docs/implementation-requirements/06-internal-architecture.md @@ -121,6 +121,7 @@ Responsibilities: - Map HTTP status and D1 response failures to public exceptions - Preserve status code, raw body, errors, and messages - Create `D1BatchException` for batch partial failures +- Create `D1RawBatchException` for raw batch item failures - Ensure sensitive values are not included in exception messages ## D1Version diff --git a/docs/implementation-requirements/07-testing-requirements.md b/docs/implementation-requirements/07-testing-requirements.md index b370ab7..fbee5ae 100644 --- a/docs/implementation-requirements/07-testing-requirements.md +++ b/docs/implementation-requirements/07-testing-requirements.md @@ -165,8 +165,9 @@ Error tests: - HTTP 200 with top-level `success=false` throws D1 exception - Query result `success=false` throws `D1QueryException` - Batch partial failure throws `D1BatchException` +- Raw batch item failure throws `D1RawBatchException` - Non-JSON error body is preserved in `rawBody()` -- JSON parse failure throws `D1ApiException` +- JSON parse failure throws `D1ApiException` with the attempted operation - Network failure throws `D1TransportException` - Timeout throws `D1TimeoutException` diff --git a/docs/implementation-requirements/10-acceptance-checklist.md b/docs/implementation-requirements/10-acceptance-checklist.md index 9c23156..d9a913c 100644 --- a/docs/implementation-requirements/10-acceptance-checklist.md +++ b/docs/implementation-requirements/10-acceptance-checklist.md @@ -66,6 +66,7 @@ - [ ] 5xx maps to `D1ApiException`. - [ ] Query result failure maps to `D1QueryException`. - [ ] Batch partial failure maps to `D1BatchException`. +- [ ] Raw batch item failure maps to `D1RawBatchException`. - [ ] Mapping failure maps to `D1MappingException`. - [ ] Timeout maps to `D1TimeoutException`. - [ ] Transport failure maps to `D1TransportException`. diff --git a/docs/requirements/phase-04-response-errors-mapping.md b/docs/requirements/phase-04-response-errors-mapping.md index 6bc85b0..4fe93c2 100644 --- a/docs/requirements/phase-04-response-errors-mapping.md +++ b/docs/requirements/phase-04-response-errors-mapping.md @@ -35,7 +35,8 @@ D1Exception │ ├─ D1AuthorizationException │ ├─ D1RateLimitException │ ├─ D1QueryException - │ └─ D1BatchException + │ ├─ D1BatchException + │ └─ D1RawBatchException ├─ D1MappingException ├─ D1TimeoutException └─ D1TransportException @@ -51,6 +52,7 @@ D1Exception - HTTP 5xx: `D1ApiException` - Query result `success=false`: `D1QueryException` - Batch result with any failed item: `D1BatchException` +- Raw batch result with any failed item: `D1RawBatchException` - Mapping failure: `D1MappingException` - Timeout: `D1TimeoutException` - Network failure: `D1TransportException` diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1Client.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1Client.java index 13792c0..856792b 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/D1Client.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1Client.java @@ -377,7 +377,7 @@ public List batch(List queries) { if (!isSuccessfulStatus(response.statusCode())) { throw exceptionFactory.httpException(response, D1Operation.BATCH, null); } - D1ApiResponseDto apiResponse = parseApiResponse(response); + D1ApiResponseDto apiResponse = parseApiResponse(response, D1Operation.BATCH); if (apiResponse.success != null && !apiResponse.success) { throw exceptionFactory.topLevelFailure( response.statusCode(), response.body(), D1Operation.BATCH, apiResponse, null); @@ -455,7 +455,7 @@ public List rawBatch(List queries) { if (!isSuccessfulStatus(response.statusCode())) { throw exceptionFactory.httpException(response, D1Operation.RAW_BATCH, null); } - D1RawApiResponseDto apiResponse = parseRawApiResponse(response); + D1RawApiResponseDto apiResponse = parseRawApiResponse(response, D1Operation.RAW_BATCH); if (apiResponse.success != null && !apiResponse.success) { throw exceptionFactory.topLevelRawFailure( response.statusCode(), response.body(), D1Operation.RAW_BATCH, apiResponse, null); @@ -497,7 +497,7 @@ private D1Result executeSingle(D1Query query, D1Operation operation) { if (!isSuccessfulStatus(response.statusCode())) { throw exceptionFactory.httpException(response, operation, query.sql()); } - D1ApiResponseDto apiResponse = parseApiResponse(response); + D1ApiResponseDto apiResponse = parseApiResponse(response, operation); if (apiResponse.success != null && !apiResponse.success) { throw exceptionFactory.topLevelFailure( response.statusCode(), response.body(), operation, apiResponse, query.sql()); @@ -515,7 +515,7 @@ private D1RawResult executeRawSingle(D1Query query) { if (!isSuccessfulStatus(response.statusCode())) { throw exceptionFactory.httpException(response, D1Operation.RAW, query.sql()); } - D1RawApiResponseDto apiResponse = parseRawApiResponse(response); + D1RawApiResponseDto apiResponse = parseRawApiResponse(response, D1Operation.RAW); if (apiResponse.success != null && !apiResponse.success) { throw exceptionFactory.topLevelRawFailure( response.statusCode(), response.body(), D1Operation.RAW, apiResponse, query.sql()); @@ -527,13 +527,15 @@ private D1RawResult executeRawSingle(D1Query query) { return result; } - private D1ApiResponseDto parseApiResponse(D1HttpResponse response) { + private D1ApiResponseDto parseApiResponse( + D1HttpResponse response, + D1Operation operation) { try { return responseParser.parseApiResponse(response.body()); } catch (JsonProcessingException e) { throw new D1ApiException( "D1 API response was not valid JSON", - null, + operation, response.statusCode(), response.body(), Collections.emptyList(), @@ -541,13 +543,15 @@ private D1ApiResponseDto parseApiResponse(D1HttpResponse response) { } } - private D1RawApiResponseDto parseRawApiResponse(D1HttpResponse response) { + private D1RawApiResponseDto parseRawApiResponse( + D1HttpResponse response, + D1Operation operation) { try { return responseParser.parseRawApiResponse(response.body()); } catch (JsonProcessingException e) { throw new D1ApiException( "D1 API response was not valid JSON", - null, + operation, response.statusCode(), response.body(), Collections.emptyList(), diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/D1RawBatchException.java b/src/main/java/io/github/xxvw/cloudflare/d1/D1RawBatchException.java new file mode 100644 index 0000000..c5b4350 --- /dev/null +++ b/src/main/java/io/github/xxvw/cloudflare/d1/D1RawBatchException.java @@ -0,0 +1,57 @@ +package io.github.xxvw.cloudflare.d1; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * D1 raw batch failure. + */ +public final class D1RawBatchException extends D1ApiException { + /** Index of the first failed raw batch item. */ + private final int failedIndex; + /** Raw result items returned by the API. */ + private final List partialResults; + + /** + * Creates a raw batch exception. + * + * @param statusCode HTTP status code + * @param rawBody raw HTTP response body + * @param errors API errors for the failed item + * @param messages API messages for the failed item + * @param failedIndex index of the first failed raw batch item + * @param partialResults raw result items returned before failure handling + */ + public D1RawBatchException( + Integer statusCode, + String rawBody, + List errors, + List messages, + int failedIndex, + List partialResults) { + super("D1 raw batch failed", D1Operation.RAW_BATCH, statusCode, rawBody, errors, messages); + this.failedIndex = failedIndex; + this.partialResults = partialResults == null + ? Collections.emptyList() + : Collections.unmodifiableList(new ArrayList<>(partialResults)); + } + + /** + * Index of the first failed raw batch item. + * + * @return failed item index + */ + public int failedIndex() { + return failedIndex; + } + + /** + * Raw result items returned by the API. + * + * @return immutable partial results + */ + public List partialResults() { + return partialResults; + } +} diff --git a/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ExceptionFactory.java b/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ExceptionFactory.java index c96a406..369abcb 100644 --- a/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ExceptionFactory.java +++ b/src/main/java/io/github/xxvw/cloudflare/d1/internal/D1ExceptionFactory.java @@ -7,6 +7,7 @@ import io.github.xxvw.cloudflare.d1.D1BatchException; import io.github.xxvw.cloudflare.d1.D1Operation; import io.github.xxvw.cloudflare.d1.D1QueryException; +import io.github.xxvw.cloudflare.d1.D1RawBatchException; import io.github.xxvw.cloudflare.d1.D1RawResult; import io.github.xxvw.cloudflare.d1.D1RateLimitException; import io.github.xxvw.cloudflare.d1.D1ResponseInfo; @@ -110,21 +111,25 @@ public D1BatchException batchFailure(int statusCode, String rawBody, List results) { - D1RawResult failed = null; - for (D1RawResult result : results) { - if (!result.success()) { - failed = result; + public D1RawBatchException rawBatchFailure( + int statusCode, + String rawBody, + List results) { + int failedIndex = 0; + for (int i = 0; i < results.size(); i++) { + if (!results.get(i).success()) { + failedIndex = i; break; } } - return new D1ApiException( - "D1 raw batch failed", - D1Operation.RAW_BATCH, + D1RawResult failed = results.isEmpty() ? null : results.get(failedIndex); + return new D1RawBatchException( statusCode, rawBody, failed == null ? Collections.emptyList() : failed.errors(), - failed == null ? Collections.emptyList() : failed.messages()); + failed == null ? Collections.emptyList() : failed.messages(), + failedIndex, + results); } private D1ApiException statusException( diff --git a/src/test/java/io/github/xxvw/cloudflare/d1/D1AsyncClientTest.java b/src/test/java/io/github/xxvw/cloudflare/d1/D1AsyncClientTest.java index ef81e32..be61732 100644 --- a/src/test/java/io/github/xxvw/cloudflare/d1/D1AsyncClientTest.java +++ b/src/test/java/io/github/xxvw/cloudflare/d1/D1AsyncClientTest.java @@ -268,17 +268,26 @@ void asyncBatchPartialFailurePropagatesBatchExceptionDetails() { @Test void executeAndRawBatchAsyncFailuresPropagatePublicExceptions() { server.enqueue(jsonError(500)); - server.enqueue(ok("{\"success\":true,\"result\":[{\"success\":false," - + "\"results\":{\"columns\":[],\"rows\":[]},\"errors\":[{\"code\":3,\"message\":\"batch\"}]}]," - + "\"errors\":[],\"messages\":[]}")); + server.enqueue(ok("{\"success\":true,\"result\":[" + + "{\"success\":true,\"results\":{\"columns\":[\"value\"],\"rows\":[[1]]}}," + + "{\"success\":false,\"results\":{\"columns\":[],\"rows\":[]}," + + "\"errors\":[{\"code\":3,\"message\":\"batch\"}]}" + + "],\"errors\":[],\"messages\":[]}")); D1AsyncClient client = testClient(); assertThatThrownBy(() -> client.executeAsync("UPDATE users SET active = ?", false).join()) .isInstanceOf(CompletionException.class) .hasCauseInstanceOf(D1ApiException.class); - assertThatThrownBy(() -> client.rawBatchAsync(D1Query.of("SELECT 1")).join()) + CompletableFuture> rawBatchFailure = + client.rawBatchAsync(D1Query.of("SELECT 1"), D1Query.of("SELECT 2")); + assertThatThrownBy(rawBatchFailure::join) .isInstanceOf(CompletionException.class) - .hasCauseInstanceOf(D1ApiException.class); + .hasCauseInstanceOf(D1RawBatchException.class); + D1RawBatchException cause = + (D1RawBatchException) rawBatchFailure.handle((value, error) -> error.getCause()).join(); + assertThat(cause.failedIndex()).isEqualTo(1); + assertThat(cause.partialResults()).hasSize(2); + assertThat(cause.errors()).extracting(D1ResponseInfo::code).containsExactly(3); } @Test diff --git a/src/test/java/io/github/xxvw/cloudflare/d1/D1ClientTest.java b/src/test/java/io/github/xxvw/cloudflare/d1/D1ClientTest.java index 222f322..6fb9e23 100644 --- a/src/test/java/io/github/xxvw/cloudflare/d1/D1ClientTest.java +++ b/src/test/java/io/github/xxvw/cloudflare/d1/D1ClientTest.java @@ -187,6 +187,36 @@ void emptyResultArrayReturnsEmptyRowsAndInvalidJsonThrowsApiException() { exception -> assertThat(exception.rawBody()).contains("not-json")); } + @Test + void invalidJsonResponsesExposeTheAttemptedOperationAndKeepMessagesSanitized() { + String sensitiveBody = "not-json test-token sensitive-param"; + for (int i = 0; i < 5; i++) { + server.enqueue(ok(sensitiveBody)); + } + D1Client client = testClient(); + + assertInvalidJsonOperation( + () -> client.query("SELECT * FROM users WHERE secret = ?", "sensitive-param"), + D1Operation.QUERY, + sensitiveBody); + assertInvalidJsonOperation( + () -> client.execute("UPDATE users SET secret = ?", "sensitive-param"), + D1Operation.EXECUTE, + sensitiveBody); + assertInvalidJsonOperation( + () -> client.batch(D1Query.of("SELECT ?", "sensitive-param")), + D1Operation.BATCH, + sensitiveBody); + assertInvalidJsonOperation( + () -> client.raw("SELECT ?", "sensitive-param"), + D1Operation.RAW, + sensitiveBody); + assertInvalidJsonOperation( + () -> client.rawBatch(D1Query.of("SELECT ?", "sensitive-param")), + D1Operation.RAW_BATCH, + sensitiveBody); + } + @Test void responseMessagesAndBatchListResultsAreImmutable() { server.enqueue(ok("{\"success\":true,\"result\":[{\"success\":true,\"results\":[],\"meta\":{}," @@ -407,16 +437,51 @@ void mapsRawTopLevelAndResultFailures() { server.enqueue(ok("{\"success\":true,\"result\":[{\"success\":false," + "\"results\":{\"columns\":[],\"rows\":[]},\"errors\":[{\"code\":2,\"message\":\"raw\"}]}]," + "\"errors\":[],\"messages\":[]}")); - server.enqueue(ok("{\"success\":true,\"result\":[{\"success\":false," - + "\"results\":{\"columns\":[],\"rows\":[]},\"errors\":[{\"code\":3,\"message\":\"batch\"}]}]," - + "\"errors\":[],\"messages\":[]}")); + server.enqueue(ok("{\"success\":true,\"result\":[" + + "{\"success\":true,\"results\":{\"columns\":[\"value\"],\"rows\":[[1]]}}," + + "{\"success\":false,\"results\":{\"columns\":[],\"rows\":[]}," + + "\"errors\":[{\"code\":3,\"message\":\"test-token\"}]," + + "\"messages\":[{\"code\":4,\"message\":\"sensitive-param\"}]}," + + "{\"success\":false,\"results\":{\"columns\":[],\"rows\":[]}," + + "\"errors\":[{\"code\":5,\"message\":\"later\"}]}" + + "],\"errors\":[],\"messages\":[]}")); D1Client client = testClient(); assertThatThrownBy(() -> client.raw("SELECT 1")).isInstanceOf(D1ApiException.class); assertThatThrownBy(() -> client.raw("SELECT 1")).isInstanceOf(D1QueryException.class); assertThatThrownBy(() -> client.rawBatch(D1Query.of("SELECT 1"))) + .isInstanceOfSatisfying(D1RawBatchException.class, exception -> { + assertThat(exception.operation()).contains(D1Operation.RAW_BATCH); + assertThat(exception.statusCode()).hasValue(200); + assertThat(exception.failedIndex()).isEqualTo(1); + assertThat(exception.partialResults()).hasSize(3); + assertThat(exception.partialResults().get(0).success()).isTrue(); + assertThat(exception.errors()).extracting(D1ResponseInfo::code).containsExactly(3); + assertThat(exception.messages()).extracting(D1ResponseInfo::code).containsExactly(4); + assertThat(exception.getMessage()) + .isEqualTo("D1 raw batch failed") + .doesNotContain("test-token") + .doesNotContain("sensitive-param"); + }); + } + + @Test + void rawBatchHttpAndTopLevelFailuresKeepExistingExceptionMapping() { + server.enqueue(jsonError(401)); + server.enqueue(ok("{\"success\":false," + + "\"errors\":[{\"code\":1,\"message\":\"top\"}],\"messages\":[]}")); + D1Client client = testClient(); + + assertThatThrownBy(() -> client.rawBatch(D1Query.of("SELECT 1"))) + .isExactlyInstanceOf(D1AuthenticationException.class) .isInstanceOfSatisfying(D1ApiException.class, - exception -> assertThat(exception.getMessage()).isEqualTo("D1 raw batch failed")); + exception -> assertThat(exception.operation()).contains(D1Operation.RAW_BATCH)); + assertThatThrownBy(() -> client.rawBatch(D1Query.of("SELECT 1"))) + .isExactlyInstanceOf(D1ApiException.class) + .isInstanceOfSatisfying(D1ApiException.class, exception -> { + assertThat(exception.operation()).contains(D1Operation.RAW_BATCH); + assertThat(exception.errors()).extracting(D1ResponseInfo::code).containsExactly(1); + }); } @Test @@ -612,4 +677,22 @@ private D1Client testClient(D1RetryPolicy retryPolicy) { .build(); } + private static void assertInvalidJsonOperation( + Runnable operation, + D1Operation expectedOperation, + String sensitiveBody) { + assertThatThrownBy(operation::run) + .isExactlyInstanceOf(D1ApiException.class) + .isInstanceOfSatisfying(D1ApiException.class, exception -> { + assertThat(exception.operation()).contains(expectedOperation); + assertThat(exception.statusCode()).hasValue(200); + assertThat(exception.rawBody()).contains(sensitiveBody); + assertThat(exception.getMessage()) + .isEqualTo("D1 API response was not valid JSON") + .doesNotContain("test-token") + .doesNotContain("sensitive-param") + .doesNotContain("SELECT"); + }); + } + } diff --git a/src/test/java/io/github/xxvw/cloudflare/d1/D1ExceptionTest.java b/src/test/java/io/github/xxvw/cloudflare/d1/D1ExceptionTest.java index 088359a..a5f5f2e 100644 --- a/src/test/java/io/github/xxvw/cloudflare/d1/D1ExceptionTest.java +++ b/src/test/java/io/github/xxvw/cloudflare/d1/D1ExceptionTest.java @@ -5,7 +5,9 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import io.github.xxvw.cloudflare.d1.testsupport.UserRow; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import org.junit.jupiter.api.Test; class D1ExceptionTest { @@ -46,4 +48,39 @@ void mappingExceptionRowIsImmutableAndMessageDoesNotExposeRowContents() { assertThatThrownBy(() -> exception.row().put("id", 1)).isInstanceOf(UnsupportedOperationException.class); } + @Test + void rawBatchExceptionCopiesPartialResultsAndUsesSanitizedMessage() { + D1ResponseInfo error = + new D1ResponseInfo(7500, "sensitive-param", null, null, Collections.emptyMap()); + D1RawResult failed = new D1RawResult( + false, + Collections.emptyList(), + Collections.>emptyList(), + D1Meta.empty(), + Collections.emptyList(), + Collections.singletonList(error), + "test-token sensitive-param"); + List partialResults = new ArrayList<>(); + partialResults.add(failed); + + D1RawBatchException exception = new D1RawBatchException( + 200, + "test-token sensitive-param", + Collections.singletonList(error), + Collections.emptyList(), + 0, + partialResults); + partialResults.clear(); + + assertThat(exception.operation()).contains(D1Operation.RAW_BATCH); + assertThat(exception.failedIndex()).isZero(); + assertThat(exception.partialResults()).containsExactly(failed); + assertThat(exception.getMessage()) + .isEqualTo("D1 raw batch failed") + .doesNotContain("test-token") + .doesNotContain("sensitive-param"); + assertThatThrownBy(() -> exception.partialResults().add(failed)) + .isInstanceOf(UnsupportedOperationException.class); + } + }