From f1bc81c6f07dabc5182662aa42d660a1fcd939b9 Mon Sep 17 00:00:00 2001 From: Ashfaqbs <105435085+Ashfaqbs@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:56:15 +0530 Subject: [PATCH 1/2] fix(client): keep status and body when a 422 is not a validation error A 422 whose body cannot be parsed as a validation error (for example an HTML page from a gateway) surfaced as a raw Jackson parse exception and lost the status code and response body. Fall back to DoclingServeClientException, as for any other 4xx/5xx. Rebased onto main after #711 and #714: - Narrow the fallback catch to the new JsonReadException instead of a broad RuntimeException, so a failure that is not a parse failure (a broken custom deserializer) still propagates. - parseValidationError now returns Optional instead of a @Nullable, folded together with #714's no-details filter. - Move the standalone tests into AbstractDoclingServeClientTests' UnprocessableEntityResponseTests, so both Jackson backends run them, and add a case guarding the narrowed catch via a mixin-based failing deserializer for ValidationError. - Update serve-api.md and whats-new.md. Signed-off-by: Ashfaqbs <105435085+Ashfaqbs@users.noreply.github.com> --- .../serve/client/DoclingServeClient.java | 46 +++++++++++-------- .../AbstractDoclingServeClientTests.java | 41 +++++++++++++++-- .../DoclingServeJackson2ClientTests.java | 16 +++++++ .../DoclingServeJackson3ClientTests.java | 16 +++++++ docs/src/doc/docs/docling-serve/serve-api.md | 1 + docs/src/doc/docs/whats-new.md | 1 + 6 files changed, 99 insertions(+), 22 deletions(-) diff --git a/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java b/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java index 924ac734..e1b87372 100644 --- a/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java +++ b/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java @@ -362,24 +362,19 @@ protected T getResponse(HttpRequest request, HttpResponse response, Class } } - if (statusCode == 422) { - // ValidationError is deserialized leniently, so any JSON object yields one. Only a - // ValidationError with details is a validation error; anything else is a generic error. - var validationError = Optional.ofNullable(readValue(body.toString(), ValidationError.class)) - .filter(error -> !error.getErrorDetails().isEmpty()); - - if (validationError.isPresent()) { - var errorText = validationError.get() - .getErrorDetails() - .stream() - .map(ValidationErrorDetail::getMessage) - .filter(Objects::nonNull) - .collect(Collectors.joining("\n")); - - throw new ValidationException( - validationError.get(), "An error occurred while making %s request to %s:\n%s".formatted(request.method(), request.uri(), errorText) - ); - } + var validationError = statusCode == 422 ? parseValidationError(body.toString()) : Optional.empty(); + + if (validationError.isPresent()) { + var errorText = validationError.get() + .getErrorDetails() + .stream() + .map(ValidationErrorDetail::getMessage) + .filter(Objects::nonNull) + .collect(Collectors.joining("\n")); + + throw new ValidationException( + validationError.get(), "An error occurred while making %s request to %s:\n%s".formatted(request.method(), request.uri(), errorText) + ); } throw new DoclingServeClientException("An error occurred: %s".formatted(body.toString()), statusCode, body.toString()); @@ -397,6 +392,21 @@ protected T getResponse(HttpRequest request, HttpResponse response, Class } } + // A 422 from something other than docling-serve (e.g. a gateway) may not carry a validation body; + // fall back to the generic error so the status code and body are not lost to a parse failure. + // ValidationError is deserialized leniently, so any JSON object parses into one - only a + // ValidationError with details is treated as an actual validation error. + private Optional parseValidationError(String body) { + try { + return Optional.ofNullable(readValue(body, ValidationError.class)) + .filter(error -> !error.getErrorDetails().isEmpty()); + } + catch (JsonReadException e) { + LOG.debug("422 response body is not a validation error", e); + return Optional.empty(); + } + } + @Override public HealthCheckResponse health() { return this.healthOps.health(); diff --git a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java index a43dcf05..7c069b1a 100644 --- a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java +++ b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java @@ -172,6 +172,17 @@ public void testFailed(ExtensionContext context, @Nullable Throwable cause) { */ protected abstract DoclingServeApi getDoclingClientWithFailingDeserializer(); + /** + * A client, built without {@code prettyPrint()}, whose JSON mapper throws {@code IllegalStateException} with + * the message {@code boom} specifically when deserializing a {@link ValidationError}, through a mixin rather + * than a deserializer registered for the type directly, since {@link ValidationError} has a builder (see + * {@link #getDoclingClientWithFailingDeserializer()}). + * + *

Used to confirm that a failure of {@code parseValidationError} that is not a {@link JsonReadException}, + * such as a broken custom deserializer, is not swallowed and re-reported as a generic 422. + */ + protected abstract DoclingServeApi getDoclingClientWithFailingValidationErrorDeserializer(); + protected DoclingServeApi getDoclingClient(boolean requiresAuth) { return getDoclingClient(requiresAuth, false); } @@ -580,7 +591,7 @@ void resetStubs() { "{\"detail\":[]}" }) void jsonBodyWithoutValidationDetailsKeepsStatusAndBody(String body) { - stubHealth(body); + stubHealth("application/json", body); assertThatThrownBy(() -> getDoclingClient(false, true).health()) .isNotInstanceOf(ValidationException.class) @@ -591,7 +602,7 @@ void jsonBodyWithoutValidationDetailsKeepsStatusAndBody(String body) { @Test void jsonBodyWithValidationDetailsIsAValidationException() { - stubHealth("{\"detail\":[{\"type\":\"missing\",\"loc\":[\"body\",\"sources\"],\"msg\":\"Field required\"}]}"); + stubHealth("application/json", "{\"detail\":[{\"type\":\"missing\",\"loc\":[\"body\",\"sources\"],\"msg\":\"Field required\"}]}"); assertThatThrownBy(() -> getDoclingClient(false, true).health()) .isNotInstanceOf(DoclingServeClientException.class) @@ -605,13 +616,35 @@ void jsonBodyWithValidationDetailsIsAValidationException() { .isEqualTo("Field required"); } - private void stubHealth(String body) { + // A 422 from something other than docling-serve, e.g. a gateway, may not carry a JSON body at all + @Test + void nonJsonBodyKeepsStatusAndBody() { + stubHealth("text/html", "Unprocessable by gateway"); + + assertThatThrownBy(() -> getDoclingClient(false, true).health()) + .asInstanceOf(InstanceOfAssertFactories.type(DoclingServeClientException.class)) + .returns(422, DoclingServeClientException::getStatusCode) + .returns("Unprocessable by gateway", DoclingServeClientException::getResponseBody); + } + + // Guards the narrowed catch in parseValidationError: only a JsonReadException falls back to a generic + // 422, any other failure while reading the body as a ValidationError must propagate + @Test + void failureThatIsNotAParseFailureIsNotHiddenAsAGeneric422() { + stubHealth("application/json", "{\"detail\":[{\"type\":\"missing\",\"loc\":[\"body\"],\"msg\":\"Field required\"}]}"); + + assertThatThrownBy(() -> getDoclingClientWithFailingValidationErrorDeserializer().health()) + .isExactlyInstanceOf(IllegalStateException.class) + .hasMessage("boom"); + } + + private void stubHealth(String contentType, String body) { getWiremockServer().stubFor( get(urlPathEqualTo("/health")) .willReturn( aResponse() .withStatus(422) - .withHeader("Content-Type", "application/json") + .withHeader("Content-Type", contentType) .withBody(body) ) ); diff --git a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson2ClientTests.java b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson2ClientTests.java index 1d83aa71..150f7914 100644 --- a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson2ClientTests.java +++ b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson2ClientTests.java @@ -8,11 +8,13 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.json.JsonMapper; import com.fasterxml.jackson.databind.module.SimpleModule; import com.github.tomakehurst.wiremock.WireMockServer; import ai.docling.serve.api.DoclingServeApi; +import ai.docling.serve.api.validation.ValidationError; /** * Integration tests for {@link DoclingServeJackson2Client}. @@ -68,10 +70,24 @@ protected DoclingServeApi getDoclingClientWithFailingDeserializer() { .build(); } + @Override + protected DoclingServeApi getDoclingClientWithFailingValidationErrorDeserializer() { + return DoclingServeJackson2Client.builder() + .baseUrl(wireMockServer.baseUrl()) + .jsonParser(JsonMapper.builder().addMixIn(ValidationError.class, FailingValidationErrorMixIn.class)) + .build(); + } + static class FailingDeserializer extends JsonDeserializer { @Override public T deserialize(JsonParser parser, DeserializationContext context) { throw new IllegalStateException("boom"); } } + + // A mixin, not a deserializer registered for ValidationError directly, since Jackson does not consult one + // registered for a type that has a builder over the builder (see FailingDeserializer above) + @JsonDeserialize(using = FailingDeserializer.class) + abstract static class FailingValidationErrorMixIn { + } } diff --git a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson3ClientTests.java b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson3ClientTests.java index 7ef9a08c..e5d3a5f5 100644 --- a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson3ClientTests.java +++ b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/DoclingServeJackson3ClientTests.java @@ -5,6 +5,7 @@ import tools.jackson.core.JsonParser; import tools.jackson.databind.DeserializationContext; import tools.jackson.databind.ValueDeserializer; +import tools.jackson.databind.annotation.JsonDeserialize; import tools.jackson.databind.json.JsonMapper; import tools.jackson.databind.module.SimpleModule; @@ -14,6 +15,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import ai.docling.serve.api.DoclingServeApi; +import ai.docling.serve.api.validation.ValidationError; /** * Integration tests for {@link DoclingServeClient}. @@ -69,10 +71,24 @@ protected DoclingServeApi getDoclingClientWithFailingDeserializer() { .build(); } + @Override + protected DoclingServeApi getDoclingClientWithFailingValidationErrorDeserializer() { + return DoclingServeJackson3Client.builder() + .baseUrl(wireMockServer.baseUrl()) + .jsonParser(JsonMapper.builder().addMixIn(ValidationError.class, FailingValidationErrorMixIn.class)) + .build(); + } + static class FailingDeserializer extends ValueDeserializer { @Override public T deserialize(JsonParser parser, DeserializationContext context) { throw new IllegalStateException("boom"); } } + + // A mixin, not a deserializer registered for ValidationError directly, since Jackson does not consult one + // registered for a type that has a builder over the builder (see FailingDeserializer above) + @JsonDeserialize(using = FailingDeserializer.class) + abstract static class FailingValidationErrorMixIn { + } } diff --git a/docs/src/doc/docs/docling-serve/serve-api.md b/docs/src/doc/docs/docling-serve/serve-api.md index 22a7b919..5a1cf7a0 100644 --- a/docs/src/doc/docs/docling-serve/serve-api.md +++ b/docs/src/doc/docs/docling-serve/serve-api.md @@ -267,6 +267,7 @@ you use. The reference client throws standard Java exceptions for HTTP and I/O f In the case of a request validation error (i.e. `docling-serve` throws a `422` error), the docling-java API will throw an `ai.docling.serve.api.validation.ValidationException` which can be caught and inspected. A `422` response that carries no validation details, such as one produced by a gateway or proxy in front of `docling-serve`, is reported as a `DoclingServeClientException`, like any other error response, so that the status code and the response body remain available. +This also covers a `422` body that cannot be read as JSON at all, such as an HTML or plain-text error page from a gateway: it is treated the same as a body with no validation details, not lost to a parse failure. ## Logging and builders diff --git a/docs/src/doc/docs/whats-new.md b/docs/src/doc/docs/whats-new.md index 5ee85cef..0c149897 100644 --- a/docs/src/doc/docs/whats-new.md +++ b/docs/src/doc/docs/whats-new.md @@ -34,6 +34,7 @@ Docling Java {{ gradle.project_version }} includes important breaking changes, a * **Custom `Executor` for async operations** — The async methods (`convertSourceAsync`, `convertSourceBatchAsync`, `convertFilesAsync`, `chunkSourceWith*ChunkerAsync`, ...) used to run on `CompletableFuture`'s default executor (usually `ForkJoinPool.commonPool()`). A new `asyncExecutor(Executor)` builder method lets you run them (task submission, status polling and result retrieval) on your own executor instead, e.g. a virtual-thread executor or one managed by your framework. When not set, the behavior is unchanged. The client never shuts the executor down. * **Logging a response that is not JSON no longer hides the error** — With both `logResponses()` and `prettyPrint()` enabled, a response body that is not JSON, such as an error page from a gateway or proxy in front of `docling-serve`, made the client fail with a raw Jackson exception while it was logging the response, so the status code and body were lost. The body is now logged as it is, and the request reports the usual `DoclingServeClientException`. The `readValue(String, Class)` method that subclasses of `DoclingServeClient` implement must now throw the new `JsonReadException`, with the failure of the JSON library as its cause, when the text is not valid JSON or does not match the expected type. * **A `422` response without validation details is a `DoclingServeClientException`** — A `422` response whose body is a JSON object without any validation details, such as `{}`, `{"detail": []}` or an error object from a gateway or proxy in front of `docling-serve`, used to throw a `ValidationException` with an empty `ValidationError` and a blank message. It now throws a `DoclingServeClientException` with the status code and the response body, like any other error response. Only a `422` response that carries validation details throws a `ValidationException`. +* **A `422` response whose body is not JSON at all no longer loses its status code and body** — A `422` from something other than `docling-serve`, such as a gateway that returns an HTML or plain-text error page, made `readValue` throw before the client could fall back, replacing the real error with a raw parse failure. That parse failure (a `JsonReadException`) is now caught and treated the same as a `422` without validation details: a `DoclingServeClientException` carrying the status code and the response body. A failure that is not a `JsonReadException`, such as a broken custom deserializer, still propagates unchanged. ### 0.6.6 From 4aedc5bd0670547c48d7c8679d1bc6dc9abbff9d Mon Sep 17 00:00:00 2001 From: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com> Date: Sun, 4 Oct 2026 09:43:52 +0530 Subject: [PATCH 2/2] fix(client): use Optional map/orElseGet chain and guard null detail bodies Addresses edeandrea's remaining review notes on #704: getResponse now throws validationError.map(...).orElseGet(...) instead of isPresent()/ get(), parenthesizes the ternary condition, and computes body.toString() once as responseBody. Also adds "null" and {"detail":null} cases to jsonBodyWithoutValidationDetailsKeepsStatusAndBody, which guard the Optional.ofNullable in parseValidationError against regressing to an unnoticed NPE. Signed-off-by: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com> --- .../serve/client/DoclingServeClient.java | 28 ++++++++----------- .../AbstractDoclingServeClientTests.java | 4 ++- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java b/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java index e1b87372..c99b921c 100644 --- a/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java +++ b/docling-serve/docling-serve-client/src/main/java/ai/docling/serve/client/DoclingServeClient.java @@ -362,22 +362,18 @@ protected T getResponse(HttpRequest request, HttpResponse response, Class } } - var validationError = statusCode == 422 ? parseValidationError(body.toString()) : Optional.empty(); - - if (validationError.isPresent()) { - var errorText = validationError.get() - .getErrorDetails() - .stream() - .map(ValidationErrorDetail::getMessage) - .filter(Objects::nonNull) - .collect(Collectors.joining("\n")); - - throw new ValidationException( - validationError.get(), "An error occurred while making %s request to %s:\n%s".formatted(request.method(), request.uri(), errorText) - ); - } - - throw new DoclingServeClientException("An error occurred: %s".formatted(body.toString()), statusCode, body.toString()); + var responseBody = body.toString(); + var validationError = (statusCode == 422) ? parseValidationError(responseBody) : Optional.empty(); + + throw validationError + .map(error -> new ValidationException( + error, "An error occurred while making %s request to %s:\n%s".formatted( + request.method(), request.uri(), error.getErrorDetails() + .stream() + .map(ValidationErrorDetail::getMessage) + .filter(Objects::nonNull) + .collect(Collectors.joining("\n"))))) + .orElseGet(() -> new DoclingServeClientException("An error occurred: %s".formatted(responseBody), statusCode, responseBody)); } if (StreamResponse.class.equals(expectedReturnType)) { diff --git a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java index 7c069b1a..d67d38fb 100644 --- a/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java +++ b/docling-serve/docling-serve-client/src/test/java/ai/docling/serve/client/AbstractDoclingServeClientTests.java @@ -588,7 +588,9 @@ void resetStubs() { @ValueSource(strings = { "{\"error\":\"gateway says no\"}", "{}", - "{\"detail\":[]}" + "{\"detail\":[]}", + "null", + "{\"detail\":null}" }) void jsonBodyWithoutValidationDetailsKeepsStatusAndBody(String body) { stubHealth("application/json", body);