Repository navigation
SOLR-16398: Return 400 instead of 500 when a v2 request body cannot be parsed - #5285
Draft
nick-boss-tech wants to merge 3 commits into
Draft
nick-boss-tech wants to merge 3 commits into
nick-boss-tech wants to merge 3 commits into
Conversation
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
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.
🤖 AI text below 🤖 (reviewed by Nick Shanin)
Draft status is due to Nick Shanin's open-PR cap; the change is complete and ready for review.
Jira: SOLR-16398. This defect was found by the new request-handling coverage on PR #5284: its
testExceptionexpects a 400 from the migrated rebalance-leaders endpoint for a body carrying non-numeric values, and the endpoint answers 500. The same failure reproduces on endpoints already migrated onmain, so the fix is split out here rather than added to that PR.What happens today
A v2 request body that Jackson cannot parse or convert is reported as a 500 server error, although the fault is in the client's request.
When a JSON body carries a value of the wrong type for a request-body field (for example a string where an integer is required), deserialization fails before the resource method runs. The resulting Jackson exception has no dedicated handling, so it falls through to
CatchAllExceptionMapperand is answered as a 500. The pre-migration APIs answered the equivalent bad input with a 400.What this change does
The JSON message-body reader converts request-entity parse and conversion failures into a 400 as it reads the body, in the standard v2 error format.
In
MessageBodyReaders, the Jackson provider that reads v2 request bodies now catchesJsonParseExceptionandMismatchedInputExceptionthrown while reading the entity and rethrows aSolrExceptionwithErrorCode.BAD_REQUESTand the message "Invalid request body: " followed by the parser's description of the problem. Placing the conversion in the reader scopes it to client input by construction: Jackson failures that indicate a server-side problem (an invalid type definition, or a failure writing a response) are not caught and keep their existing 500 handling, and unit tests pin that distinction. Every v2 endpoint that takes a JSON body gets the corrected status, including the endpoints already migrated to JAX-RS onmain(create collection, split shard) and the ones migrating in sibling PRs (rebalance leaders, migrate docs, move replica). Regression tests post an unconvertible body and a malformed-JSON body to the create-collection endpoint and require a 400 whose message names the problem value.Proof
The failing requests return 500 on the base code and 400 with the fix; the surrounding suites stay green.
On the unmodified
maincode, the create-collection endpoint returns 500 for{"name": ..., "replicationFactor": "abc"}, with anInvalidFormatExceptionin the server log; this was recorded before the fix was written. With this change the same requests return 400. Full gate at42c055a853: tidy clean, Error Prone compile passes,V2ApiIntegrationTestpasses 15 of 15 (including both request-body tests, which also assert the error message),PostRequestLoggingFilterTestpasses 14 of 14 (including the tests that definition and output failures remain server errors),TestApiFramework3 of 3,TestCollectionAPIs2 of 2,JerseyApplicationSharingTest1 of 1,SolrJacksonMapperTest3 of 3, and:solr:core:check -x testpasses. The failing case on PR #5284 (rebalance leaders) exercises the same reader through the same code path; that PR's test goes green once this change is under it.Limits
Only failures while reading a JSON request entity change status.
Malformed JSON syntax and values that cannot be converted to a body field's type become 400s. Everything else is unchanged: exceptions thrown by resource code after a body has been parsed, Jackson definition and output failures, and request bodies in formats that do not pass through the JSON reader (Javabin, XML) all keep their current handling and status codes. The error message includes the parser's description of the problem (the field and the expected type), matching what the pre-migration APIs surfaced for bad input.
Nick Shanin directed the work and takes responsibility for this contribution.