Repository navigation
fix(client): log a non-JSON response body as it is - #711
Merged
edeandrea merged 1 commit intoSep 29, 2026
Merged
Conversation
edeandrea
added
bug
edeandrea
enabled auto-merge (squash)
September 29, 2026 19:33
With both logResponses() and prettyPrint() enabled, logging a response that is not JSON, such as an error page from a gateway in front of docling-serve, failed with a raw Jackson exception before the status code was checked, so the status code and body were lost. readValue now throws JsonReadException, a library-neutral exception that is not a DoclingServeClientException, for text that is not valid JSON or does not match the requested type. logResponse catches only that and logs the body unchanged; any other failure still propagates. References docling-project#709 Signed-off-by: Eric Deandrea <eric.deandrea@ibm.com>
edeandrea
force-pushed
the
fix/709-log-response-non-json-body
branch
from
September 29, 2026 19:34
384e614 to
4890ab8
Compare
:java_duke: JaCoCo coverage report
|
|
||||||||||||||
|
HTML test reports are available as workflow artifacts (zipped HTML). • Download: Artifacts for this run |
|
HTML test reports are available as workflow artifacts (zipped HTML). • Download: Artifacts for this run |
This was referenced Sep 29, 2026
Ashfaqbs
added a commit
to Ashfaqbs/docling-java
that referenced
this pull request
Sep 30, 2026
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>
edeandrea
pushed a commit
to Ashfaqbs/docling-java
that referenced
this pull request
Oct 1, 2026
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>
edeandrea
pushed a commit
to Ashfaqbs/docling-java
that referenced
this pull request
Oct 1, 2026
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>
edeandrea
pushed a commit
to Ashfaqbs/docling-java
that referenced
this pull request
Oct 2, 2026
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>
edeandrea
pushed a commit
to Ashfaqbs/docling-java
that referenced
this pull request
Oct 5, 2026
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>
edeandrea
pushed a commit
that referenced
this pull request
Oct 5, 2026
* 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<ValidationError> 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> * 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> --------- Signed-off-by: Ashfaqbs <105435085+Ashfaqbs@users.noreply.github.com> Signed-off-by: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com>
Contributor
|
🎉 This issue has been resolved in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
With both
logResponses()andprettyPrint()enabled,DoclingServeClient.logResponse(...)tried to parse every response body as JSON so it could pretty-print it. A body that is not JSON, such as an error page from a gateway or proxy in front of docling-serve, made that parse throw. The exception replaced the real error, so the caller got a raw Jackson exception and the status code and response body were lost.The body is now logged as it is, and the request reports the usual
DoclingServeClientException.How
JsonReadException(a plainRuntimeException, deliberately not aDoclingServeClientException, since reading a text as JSON says nothing about an HTTP status code or response body). It is whatreadValuereports for text that is not valid JSON or does not match the requested type, whichever JSON library is used. The library's own exception is its cause.readValue(JacksonExceptionon Jackson 3,JsonProcessingExceptionon Jackson 2) and throwsJsonReadException. Any other failure, such as one thrown by a custom deserializer, propagates unchanged.logResponsecatches onlyJsonReadExceptionand logs the body unchanged.The base class cannot name a Jackson type, because only one Jackson version may be on the classpath. A library-neutral exception lets it catch precisely the failure it cares about without a broad
catch (RuntimeException), and without an extra abstract method on every subclass.Behavior changes to be aware of
readValueused to throw a bareRuntimeExceptionon Jackson 2 and letJacksonExceptionescape on Jackson 3. Both are nowJsonReadException. Code that caughtJacksonExceptionfrom the Jackson 3 client would stop matching.DoclingServeClientmust now throwJsonReadExceptionfromreadValuefor those failures. This is a contract change with no compile error; it is noted inwhats-new.md.HttpOperations.javashows a larger diff than the one@throwsline I changed: Spotless is ratcheted fromorigin/main, so touching the file re-aligned four@paramblocks that were already misformatted..gitignoregains.explyt/.Tests
New nested
NonJsonResponseTestsinAbstractDoclingServeClientTests, so both Jackson backends inherit them. Its clients already havelogResponsesandprettyPrintenabled, which is the path the bug was in. Eight tests per backend:readValuethrows exactlyJsonReadException(and not aDoclingServeClientException) for non-JSON text and for JSON of another shape.IllegalStateExceptionpropagates, both fromreadValuedirectly and while a response is being logged.I checked that the tests can fail by mutating the code and confirming they turn red on both backends: restoring the old unguarded
readValueinlogResponse, widening the catch inlogResponse, making a backend wrap every exception, making either backend stop wrapping, and re-couplingJsonReadExceptiontoDoclingServeClientException.spotlessCheckand the full:docling-serve-client:testsuite pass locally (207 tests, 0 failures).Related
Closes #709
Found while reviewing #704, which adds a fallback for a
422whose body is not a validation error. That fallback was bypassed for clients with both options enabled, because this code threw first. #704 can catchJsonReadExceptionin the base class instead of a broadRuntimeException.