Jackson 2.8

[joschi]

Refs #1627.

CC: @cowtowncoder

[joschi]

@dropwizard/committers I'm still unhappy about having to change this. Any ideas?

[nickbabcock]

Gah, that's because the exception message starts with "No suitable constructor", which is one of our heuristics for a programmer error 😿

We'll have to change the heuristics (and not the test case).

[cowtowncoder]

Not sure if this helps, but there are couple of other JsonMappingException subtypes available, beside ones already checked for (InvalidFormatException, PropertyBindingException):

• IgnoredPropertyException
• InvalidTypeIdException (added in 2.8)
• UnrecognizedPropertyException (subtype of PropertyBindingException)

Also if there is something small I could do wrt exception message I could consider changes for 2.8.2. I hope to keep messages (more) stable going forward. 2.8 changes were to try to unify messages, as well as allow more customized handling of problems via DeserializerProblemHandler extensions.

[nickbabcock]

Thanks for the tip, I'll have to take another look. I know I took a few swings at it in #1544 and #1527

[jplock]

@joschi @nickbabcock do we want to merge in this PR and then revisit the exception handling in a follow-on PR?

[nickbabcock]

I don't think we need to be in a rush to merge this PR, as I'd rather this PR stays open to remind us (me) to investigate this issue

[nickbabcock]

Still scratching my head, so I've decided to do a brain dump. I think the root cause is that there is not a great way to differentiate between various errors of JsonMappingException

Examples from the JsonProcessingExceptionMapper test cases:

Client Errors :

• Expecting an object, but JSON is false throws a JsonMappingException of "Can not construct instance of foo: no boolean/Boolean-argument constructor/factory method to deserialize from boolean value (false)". This should be a client error because the wrong json type was submitted.
• Expecting a URI, but JSON is "no-scheme.com", throws a JsonMappingException of "Unexpected IOException (of type java.net.MalformedURLException): no protocol: no-scheme.com". Ideally there would be a way to reach the underlying cause, but JsonMappingException is the root. Other exceptions while deserializing can be uncovered (like DateTimeException). Looks to only affect test case returnsA400ForMalformedInputCausingIoException
• Expecting a list but given a string throws a JsonMappingException of "Can not deserialize instance of java.util.ArrayList out of VALUE_STRING token"
• Deserializing a DateTime from [1,1,1,1] throws a JsonMappingException of "Unexpected token (VALUE_NUMBER_INT), expected END_ARRAY: Expected array to end."

Server Errors:

• BrokenRepresentation is missing a constructor and the exception is a JsonMappingException "Can not construct instance of BrokenRepresentation: no suitable constructor found, can not deserialize from Object value (missing default constructor or creator, or perhaps need to add/enable type information?)"
• NonBeanImplementation doesn't contain any information on serialization and exception is a JsonMappingException "No serializer found for class NonBeanImplementation and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS)"
• Trying to construct from an interface throws a JsonMappingException of "Can not construct instance of IInterface: abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information"

These seven situations all use JsonMappingException as the root cause and this is where we had to introduce heuristics. The following code gets us pretty close, but doesn't work on the client errors mentioned above.

final JsonMappingException ex = (JsonMappingException) exception;
final Throwable cause = Throwables.getRootCause(ex);

// Exceptions that denote an error on the client side
final boolean clientCause = cause instanceof InvalidFormatException ||
    cause instanceof PropertyBindingException ||
    cause.getClass() != JsonMappingException.class;

if (!clientCause) {
    LOGGER.error("Unable to serialize or deserialize the specific type", exception);
    return Response.serverError().build();
}

[cowtowncoder]

I think I could see a few things to improve on Jackson side. URI in particular should probably internally signal IllegalArgumentException (or similar), and then bound to one of JsonMappingException subtypes, most likely InvalidFormatException. This could go in a patch since I think it's sort of a flaw at this point, underspecifying failure.

Similarly, "wrong token" style exceptions (boolean for Object) should probably be indicated differently, but they are little bit more difficult to figure out.

This is actually kind of work I was hoping to get started by changes that lead to these practical problems: reworking error handling. I have added some structure, but there is no uniformity yet. I think DW use case, as an example, could be a good driver in figuring out semantic approach to signalling failures.

Not sure how easy it is to divide things neatly into client/server buckets, however; it would seem that in many cases majority could be viewed as client problem, sending JSON that does not match expected structure.

[coveralls]

Coverage increased (+0.2%) to 82.395% when pulling 7dd667e on jackson-2.8 into 99e9d63 on master.

[nickbabcock]

This should probably be moved to the 1.1.0 section, as 1.0.0 has already shipped ;)

[jplock]

Jackson 2.8.2 is now in maven central if we want to update to that

[cowtowncoder]

Also, for versions that want to stick with 2.7, if any, there's 2.7.7 which has many/most of the same fixes. I'll try to keep 2.7 branch alive at least until end of year as it could be long-term stable branch to try to get most frameworks upgrade to (have had major hassles with Spark myself :) ).

I'd also love to help with 2.8 issues if any remain. Did 2.8.2 release to flush all pending fixes.

[nickbabcock]

Please correct me if I'm wrong, but I took a quick look at the release notes for core and databind and didn't see how the situation pointed out by an earlier comment changes. I realize the conversation fell somewhat to the wayside (my fault 😊)

[cowtowncoder]

@nickbabcock you are correct, not much has happened for those; it would be best to file separate issues since there's need for testing and some coordination on most optimal way to improve. By flushing fixes I meant ones that have been reported via issue tracker.

[nickbabcock]

Ah, I'm not sure how you want it, did you want each of the points described earlier in a separate issue in the databind repo or a single issue?

[cowtowncoder]

@nickbabcock Hmmh. How about I'll create one for improving URI / URL handling since I should be able to do it relatively easily. And if you could add another one for other aspects, wherever there is a clear path for improvement for specific Java types, and failures?

[cowtowncoder]

@nickbabcock Looks like InvalidFormatException was properly produced for java.net.URI, but NOT for java.net.URL: I fixed this for 2.8(.3) as the first step.
As soon as I get to know of other types for which same occurs they can be fixed quite easily.

[cowtowncoder]

Ok, so now there is the wider-ranging issue for jackson-databind, which I think is good and should help a lot for medium-to-long term.

But since that work has to go in 2.9 (new exception types added), I would also be interested in incremental improvements in the meantime. If there are commonly encountered failures, or even just ones triggered by tests, that could/should be improved within constraints of existing types, would be great to resolve those too.

[nickbabcock]

The immediate concern for jackson data-bind 2.8 is that the error message:

Can not construct instance of io.dropwizard.jersey.jackson.OkRepresentation: no boolean/Boolean-argument constructor/factory method to deserialize from boolean value (false)
at [Source: org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream@6228440c; line: 1, column: 1]

Gets mis-categorized as a server error as the message falls under the heuristic:

No suitable constructor found

Whereas under 2.7.6

Can not instantiate value of type [simple type, class io.dropwizard.jersey.jackson.OkRepresentation] from Boolean value (false); no single-boolean/Boolean-arg constructor/factory method
at [Source: org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream@eca3d94; line: 1, column: 1]

and in both instances the exception is the JsonMappingException, which as we've discussed is ambiguous.

So we can either:

1. Stick to 2.7 and wait until 2.9 or whenever enhanced error handling lands.
2. Update heuristics (not sure how to differentiate types of "Can not construct instance" 😕 )

[cowtowncoder]

@nickbabcock If feasible, I would look for both 2.7 (and before) and 2.8 messages, with a note mentioning which is which. I assume DW itself has no hard requirement to use 2.8.
2.9 will take its time; current release cadence has been about 6 months between releases, which would suggest next version available at around Jan 2017.

[jplock]

Closing in favor of #1749