FrankChen021 commented on code in PR #20830:
URL: https://github.com/apache/druid/pull/20830#discussion_r4467353749
##########
server/src/main/java/org/apache/druid/server/QueryResource.java:
##########
@@ -300,6 +301,10 @@ private Query<?> readQuery(
catch (ValueInstantiationException e) {
throw new BadJsonQueryException(e);
}
+ catch (JsonMappingException e) {
Review Comment:
[P2] Broad mapping catch hides definition failures
**Finding:** `JsonMappingException` also covers
`InvalidDefinitionException`, which Jackson uses when the target query type
itself is invalid, such as a registered subtype with no usable creator. Those
failures are independent of request JSON, but this catch wraps them in
`BadJsonQueryException`; `doPost` then returns HTTP 400 and skips the catch-all
warning/500 path, making a broker-side configuration or code defect look like a
client error.
**Suggestion:** Limit this conversion to input-side mapping exceptions such
as `MismatchedInputException` and `InvalidTypeIdException`, while allowing
`InvalidDefinitionException` to reach the server-error path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
