fix(client): keep status and body when a 422 body is not JSON

[Ashfaqbs]

What

DoclingServeClient.getResponse read every 422 body as a ValidationError. A 422 that does not come from docling-serve (for example an HTML or plain-text error page from a gateway or proxy) made readValue throw, so the caller got a raw parse failure and the status code and response body were lost.

Change

• parseValidationError now catches JsonReadException (added in fix(client): log a non-JSON response body as it is #711), which readValue throws when the body is not valid JSON or does not match the expected type. In that case the client falls back to a DoclingServeClientException carrying the status code and the response body, like any other error response. Any other failure, such as a broken custom deserializer, still propagates.
• parseValidationError returns Optional<ValidationError>, and fix(client): throw DoclingServeClientException for a 422 without validation details #714's no-details filter moved into it, so there is a single check. A 422 is a ValidationException only when it carries validation details. A body that is not JSON, JSON of another shape, null, or JSON without details is a DoclingServeClientException.
• getResponse throws the result of map(...).orElseGet(...) on that Optional.
• Docs: serve-api.md ("Validation errors") and whats-new.md.

Tests

The cases are in UnprocessableEntityResponseTests in AbstractDoclingServeClientTests, so they run against both the Jackson 2 and the Jackson 3 client:

• a 422 with validation details is a ValidationException
• a 422 whose JSON body has no validation details ({"error": ...}, {}, {"detail":[]}, null, {"detail":null}) is a DoclingServeClientException with the status code and body
• a 422 with an HTML body is a DoclingServeClientException with the status code and body
• a failure while reading the body that is not a JsonReadException propagates, through a client whose mapper fails on ValidationError (via a mixin)

Signed off per DCO.

[github-actions]

:java_duke: JaCoCo coverage report

Overall Project	50.77%	🟢

There is no coverage information present for the Files changed

[github-actions]

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

Test	Result
No test annotations available

• View Gradle Test Results (all modules & JDKs)

[github-actions]

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

1. Narrow the catch to JsonReadException. This relies on #711, which came out of my review of this PR. See the inline comment.
2. Move the tests into AbstractDoclingServeClientTests so both Jackson backends run them, and add a case for the narrowed catch (inline).
3. (non-blocking) Return Optional from parseValidationError instead of @Nullable (inline).
4. 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]

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

• Download: Artifacts for this run

[Ashfaqbs]

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

1. 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.
2. 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.
3. 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())).
4. 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]

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:

1. 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.
2. null bodies (inline). Nothing guards Optional.ofNullable, and {"detail":null} is worth a case.
3. 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.

[github-actions]

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

• Download: Artifacts for this run

[github-actions]

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

• Download: Artifacts for this run

[Ashfaqbs]

Thanks for the two follow-up notes. Pushed both:

1. getResponse now throws validationError.<RuntimeException>map(...).orElseGet(...) instead of isPresent()/get(), parenthesized the ternary condition, and computes body.toString() once as responseBody.
2. Added "null" and {"detail":null} to jsonBodyWithoutValidationDetailsKeepsStatusAndBody - confirmed both would turn red if Optional.ofNullable regressed to Optional.of in parseValidationError.

spotlessCheck and compileTestJava pass locally. Same as before, I don't have Docker here so I can't run AbstractDoclingServeClientTests itself (it starts docling-serve via Testcontainers in a static initializer) - relying on CI for that.

[github-actions]

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

• Download: Artifacts for this run

[edeandrea]

Thanks a lot, @Ashfaqbs, for sticking with this through several rounds, including one request that depended on #711, which didn't exist when you opened the PR. Folding #714's no-details filter into parseValidationError rather than keeping two checks was a good call, and the mixin-based failing deserializer for the propagation test was exactly right.

I re-ran everything on top of your branch. The 422 and non-JSON tests pass on both Jackson backends, and each guard is real: widening the catch, replacing Optional.ofNullable with Optional.of, an always-true filter and a wrong status code each turn tests red. I also updated the PR title and description to match what the PR does now.

Approving. Thanks again!

[github-actions]

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

• Download: Artifacts for this run

[docling-java-ops]

🎉 This issue has been resolved in v0.7.0 (Release Notes)