Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -362,27 +362,18 @@ protected <T> 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 responseBody = body.toString();
var validationError = (statusCode == 422) ? parseValidationError(responseBody) : Optional.<ValidationError>empty();

throw new DoclingServeClientException("An error occurred: %s".formatted(body.toString()), statusCode, body.toString());
throw validationError
.<RuntimeException>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)) {
Expand All @@ -397,6 +388,21 @@ protected <T> 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<ValidationError> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()}).
*
* <p>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);
}
Expand Down Expand Up @@ -577,10 +588,12 @@ void resetStubs() {
@ValueSource(strings = {
"{\"error\":\"gateway says no\"}",
"{}",
"{\"detail\":[]}"
"{\"detail\":[]}",
"null",
"{\"detail\":null}"
})
void jsonBodyWithoutValidationDetailsKeepsStatusAndBody(String body) {
stubHealth(body);
stubHealth("application/json", body);

assertThatThrownBy(() -> getDoclingClient(false, true).health())
.isNotInstanceOf(ValidationException.class)
Expand All @@ -591,7 +604,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)
Expand All @@ -605,13 +618,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", "<html>Unprocessable by gateway</html>");

assertThatThrownBy(() -> getDoclingClient(false, true).health())
.asInstanceOf(InstanceOfAssertFactories.type(DoclingServeClientException.class))
.returns(422, DoclingServeClientException::getStatusCode)
.returns("<html>Unprocessable by gateway</html>", 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)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down Expand Up @@ -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<T> extends JsonDeserializer<T> {
@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 {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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}.
Expand Down Expand Up @@ -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<T> extends ValueDeserializer<T> {
@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 {
}
}
1 change: 1 addition & 0 deletions docs/src/doc/docs/docling-serve/serve-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions docs/src/doc/docs/whats-new.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading