fix(client): keep status and body when a 422 is not a validation error by Ashfaqbs · Pull Request #704 · docling-project/docling-java

Ashfaqbs · 2026-09-25T03:26:20Z

What

DoclingServeClient.getResponse parses every 422 body as a ValidationError. If the body is not one (for example an HTML error page from a gateway or proxy in front of docling-serve), the parse failure escapes as a raw Jackson exception, and the status code and response body are lost.

Change

If the 422 body cannot be parsed as a validation error, fall back to DoclingServeClientException carrying the status code and body, the same as any other 4xx/5xx response. Real validation errors still throw ValidationException.

Tests

New DoclingServeClientErrorResponseTests (WireMock, no container needed):

a 422 with a validation body still throws ValidationException
a 422 with an HTML body throws DoclingServeClientException with status 422 and the body

Before the change the second test fails with tools.jackson.core.exc.StreamReadException; after it, both pass. spotlessCheck reports no violations in the files touched here.

Signed off per DCO.

github-actions · 2026-09-29T17:56:45Z

:java_duke: JaCoCo coverage report

Overall Project	50.75%	🟢

There is no coverage information present for the Files changed

github-actions · 2026-09-29T17:56:47Z

	Tests	Passed ✅	Skipped	Failed
Gradle Test Results (all modules & JDKs)	2236 ran	2236 passed	0 skipped	0 failed

Test	Result
No test annotations available

View Gradle Test Results (all modules & JDKs)

github-actions · 2026-09-29T17:56:48Z

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

edeandrea

Thanks for the PR, @Ashfaqbs. Falling back to DoclingServeClientException (status and body kept) when a 422 isn't a validation error is the right behavior. I tried it on both Jackson backends: an HTML page, [1, 2], an empty body and null all end up as a DoclingServeClientException with status 422 and the original body, while a real validation body still throws ValidationException.

A few things I'd like to see before this merges (details inline):

Narrow the catch to JsonReadException. This relies on #711, which came out of my review of this PR. See the inline comment.
Move the tests into AbstractDoclingServeClientTests so both Jackson backends run them, and add a case for the narrowed catch (inline).
(non-blocking) Return Optional from parseValidationError instead of @Nullable (inline).
Docs. docs/src/doc/docs/docling-serve/serve-api.md ("Validation errors") says a 422 from docling-serve makes the API throw ValidationException. With this change that is only true when the body can be read as a validation error. Otherwise it is a DoclingServeClientException carrying the status code and body. Could you add a sentence there, and a bug-fix bullet under the current version in docs/src/doc/docs/whats-new.md next to the entry from #711?

One thing that is not needed here: a JSON 422 body with no detail (for example {"error": "..."}) still parses into an empty ValidationError and gives a ValidationException with a blank message. That is pre-existing and separate from what this PR fixes, so I opened #710 for it.

github-actions · 2026-09-29T20:51:12Z

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

Ashfaqbs · 2026-09-30T01:57:22Z

Thanks for the thorough review, @edeandrea. Rebased onto main (picked up #711 and #714) and addressed all four points:

Narrowed the catch to JsonReadException. parseValidationError now catches only that, so a broken custom deserializer or any other non-parse failure propagates instead of being reported as a generic 422.
Moved the tests into AbstractDoclingServeClientTests. They're now in UnprocessableEntityResponseTests (which fix(client): throw DoclingServeClientException for a 422 without validation details #714 had already started), so both Jackson backends run them. Added the case for the narrowed catch: a getDoclingClientWithFailingValidationErrorDeserializer() hook on both backend test classes, using the mixin approach you suggested (addMixIn(ValidationError.class, FailingMixIn.class)), since ValidationError has a builder and a directly-registered deserializer wouldn't be honored - confirmed that failure propagates as IllegalStateException("boom") rather than turning into a DoclingServeClientException.
Optional<ValidationError> instead of @Nullable, and getResponse updated to match - used your suggested shape, folded together with fix(client): throw DoclingServeClientException for a 422 without validation details #714's no-details filter (Optional.ofNullable(...).filter(error -> !error.getErrorDetails().isEmpty())).
Docs: added a sentence to serve-api.md's "Validation errors" section covering an unparseable body, and a whats-new.md bullet for this fix specifically (next to fix(client): log a non-JSON response body as it is #711's and fix(client): throw DoclingServeClientException for a 422 without validation details #714's entries).

Since #714 landed first and already added the no-details filter inline, I merged that filter into parseValidationError rather than keeping two separate checks - so a 422 is now a DoclingServeClientException whether the body has no validation details, isn't JSON at all, or is JSON of some other shape, and a ValidationException only when it actually has details.

spotlessCheck passes locally. I don't have Docker available in my current environment to run the Testcontainers-backed suite here, so I'm relying on CI for that - let me know if anything comes back red.

github-actions · 2026-09-30T02:18:33Z

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

edeandrea

Thanks for the quick turnaround, @Ashfaqbs, and for folding #714's filter into parseValidationError instead of keeping two checks. All four points from the last round are in: the narrowed JsonReadException catch, the Optional return, the tests in AbstractDoclingServeClientTests (with a working mixin hook for the propagation case), and the docs. I ran the new tests on both Jackson backends and checked that they can fail: widening the catch, making it never fire, and dropping the empty-details filter each turn them red. I've resolved the three earlier threads.

What is left is small, and none of it blocks the merge:

getResponse style (inline). isPresent() followed by get() should be a fluent Optional chain, and the condition of the inline conditional needs parentheses. One suggestion covers both.
null bodies (inline). Nothing guards Optional.ofNullable, and {"detail":null} is worth a case.
The PR description still describes the first version of this change: it mentions DoclingServeClientErrorResponseTests, "no container needed", and "if the 422 body cannot be parsed". Could you update it to match what the PR does now? The catch names JsonReadException (added by #711), the tests are in UnprocessableEntityResponseTests and run through Testcontainers, and the no-details filter from #714 now lives in parseValidationError.

Once these are in, this looks good to me.

edeandrea · 2026-09-30T15:17:15Z

+      var validationError = statusCode == 422 ? parseValidationError(body.toString()) : Optional.<ValidationError>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());


Two small style items in this block, both from the project's coding standards. The suggestion below fixes both:

validationError.isPresent() followed by validationError.get() (twice) is the isPresent/get pattern. It reads better as a fluent chain that builds the exception, so getResponse throws the result of map(...).orElseGet(...). That also lets body.toString() be computed once, as responseBody, instead of three times.

The condition of the inline conditional gets parentheses: (statusCode == 422) ? ... : ....

This is the shape from my first review, before it was folded together with #714's change, so it is easy to lose. I tried the suggestion on top of your branch (nothing is pushed): it compiles, spotlessCheck passes, and the UnprocessableEntityResponseTests pass on both Jackson backends.

Suggested change

var validationError = statusCode == 422 ? parseValidationError(body.toString()) : Optional.<ValidationError>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.<ValidationError>empty();

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));

edeandrea · 2026-09-30T15:17:15Z

@@ -573,7 +584,7 @@ void resetStubs() {
        "{\"detail\":[]}"


The tests guard the narrowed catch and the empty-details filter well. One gap: nothing guards Optional.ofNullable in parseValidationError. When I replaced it with Optional.of, all the tests stayed green, so a JSON null body could start throwing an NPE unnoticed. Adding "null" here turns that red on both backends.

{"detail":null} is worth a case too. It used to surface as a JsonReadException on Jackson 2 ("errorDetails cannot be null", from the builder). With this PR it falls back to a DoclingServeClientException on both backends, and a test would keep it that way. I ran both bodies against your branch, and they pass on both Jackson backends.

Suggested change

"{\"detail\":[]}"

"{\"detail\":[]}",

"null",

"{\"detail\":null}"

github-actions · 2026-10-01T19:42:02Z

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

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 docling-project#711 and docling-project#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<ValidationError> instead of a @nullable, folded together with docling-project#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>

github-actions · 2026-10-02T20:06:30Z

HTML test reports are available as workflow artifacts (zipped HTML).

• Download: Artifacts for this run

edeandrea force-pushed the fix/validation-error-unparseable-body branch from 618755c to ce1b74f Compare September 29, 2026 17:35

edeandrea force-pushed the fix/validation-error-unparseable-body branch from ce1b74f to 7e70f96 Compare September 29, 2026 20:22

edeandrea reviewed Sep 29, 2026

View reviewed changes

edeandrea mentioned this pull request Sep 29, 2026

fix(client): throw DoclingServeClientException for a 422 without validation details #714

Merged

Ashfaqbs force-pushed the fix/validation-error-unparseable-body branch from 7e70f96 to 9a9707f Compare September 30, 2026 01:55

edeandrea reviewed Sep 30, 2026

View reviewed changes

edeandrea force-pushed the fix/validation-error-unparseable-body branch 2 times, most recently from 6d377fd to b8e0a71 Compare October 1, 2026 19:26

edeandrea force-pushed the fix/validation-error-unparseable-body branch from b8e0a71 to 2e7612b Compare October 2, 2026 19:37

-        "{\"detail\":[]}"
+        "{\"detail\":[]}",
+        "null",
+        "{\"detail\":null}"

Conversation

Ashfaqbs commented Sep 25, 2026

What

Change

Tests

Uh oh!

github-actions Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

:java_duke: JaCoCo coverage report

Uh oh!

github-actions Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 29, 2026

Uh oh!

edeandrea left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 29, 2026

Uh oh!

Ashfaqbs commented Sep 30, 2026

Uh oh!

github-actions Bot commented Sep 30, 2026

Uh oh!

edeandrea left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edeandrea Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edeandrea Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 1, 2026

Uh oh!

github-actions Bot commented Oct 2, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 29, 2026 •

edited

Loading

github-actions Bot commented Sep 29, 2026 •

edited

Loading

edeandrea left a comment •

edited

Loading

edeandrea Sep 30, 2026 •

edited

Loading