Throw InvalidFormatException instead of MismatchedInputException for ACCEPT_FLOAT_AS_INT coercion failures

[mjustin]

I noticed that when ACCEPT_FLOAT_AS_INT is disabled, the application throws MismatchedInputException. It seems InvalidFormatException would be a good fit here, with the benefit of programmatically exposing the value that failed conversion.

I noticed this since I've added some custom handling in my application for InvalidFormatException/MismatchedInputException to provide terser/more user-appropriate errors. Since this scenario throws MismatchedInputException, I cannot programmatically retrieve the JSON value that failed to serialize. If it did implement InvalidFormatException, I could use InvalidFormatException.getValue() to get the value.

Note: This same reasoning applies to other similar features, such as ALLOW_COERCION_OF_SCALARS & FAIL_ON_NULL_FOR_PRIMITIVES.

[cowtowncoder]

Thank you for suggestion. This is an area that could use improvements for sure, and unifying exceptions used would make sense. But I will have to think about this carefully -- there is also another option of com.fasterxml.jackson.core.exc.InputCoercionException (added in jackson-core 2.10) that might be even better match semantically. But I agree with the point that having access to the problematic value would be useful wrt error reporting.

[Blacklands]

It seems the same is happening when using the FAIL_ON_NULL_CREATOR_PROPERTIES and FAIL_ON_MISSING_CREATOR_PROPERTIES features for deserialization. These also cause just a MismatchedInputException to be thrown.
The information (the name and index of the attribute that was null/missing) is in the exception message, so it should be possible to throw a more specific exception that also contains these as fields, right?

For now, I suppose I will have to parse the exception message to obtain this information (which is very hacky and prone to break, I know!).

[cowtowncoder]

Some notes on this:

1. One problem in including more information is just finding suitable exception with good fields: InvalidFormatException does have "value", but that would not be very useful for FAIL_ON_xxx_PROPERTIES
2. There is PropertyBindingException base type which does have information on property in question, however, with 2 existing subtypes (IgnoredPropertyException and UnrecognizedPropertyException). These are not types to use (semantically different), but a new exception type(s) could be added

I probably won't have much time to look into this in near future, but what would eventually be useful would be simple reproduction to show some use cases: these could serve as verification and regression tests (for now should just catch existing exception, check that message contains name; these would be changed after fix).

[Blacklands]

I personally am a big fan of leveraging the type system for error handling - that's something that can be done well via exceptions.
With a "rich" exception hierarchy, the caller can decide if they want to individually handle each possible error (by having a catch clause for each exception type), and if they don't care then can just catch one of the base types higher in the hierarchy and call it a day, and no extra work is needed.
And having an exception type for each error makes it possible to include specific information about the error.

For a missing creator property, I would except something like MissingCreatorPropertyException, with a field containing the name of the missing property. Same thing with a NullCreatorPropertyException, and so on. (None of these type names is supposed to be final, just examples.)
If these new exception types derive from the exception types that are currently thrown (like MismatchedInputException, PropertyBindingException, and so on), then this also wouldn't break backwards compatibility, right? Callers which are currently catching those types can continue to do so, or they can update their code to catch the new, more specific exception types and gain access to the error information those have.

I'm not sure what I could contribute to this issue unfortunately. I can understand that it's not a very high priority thing to look into. Thanks for commenting on it, though!

Maybe here's, as an example, my personal use case with this right now

I have a REST API (using Spring Boot Web with Jackson) and I want to give clients meaningful errors when they send "garbage" JSON to me.
I'm using Spring's @ControllerAdvice functionality to catch all the Jackson exceptions and then generate error responses depending on what went wrong (e.g. missing required property).
This is made a breeze by just turning on FAIL_ON_NULL_CREATOR_PROPERTIES and the like, except that then I only get the base exceptions types in the @ControllerAdvice class, so I don't have access to the details of what went wrong (unless I parse the exception messages, which is what I had to resort to for now). Having an individual exception type for each of these errors would make this wonderfully simple.

[mjustin]

Here's a simple test for the current behavior:

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import org.junit.jupiter.api.Test;

import java.math.BigInteger;
import java.util.List;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class DisallowFloatAsIntTest {
    private ObjectMapper objectMapper = new ObjectMapper().disable(DeserializationFeature.ACCEPT_FLOAT_AS_INT);

    @Test
    public void exceptionIncludesProblematicValue() {
        testExceptionIncludesProblematicValue("5.5", Integer.class);
        testExceptionIncludesProblematicValue("5.0", Long.class);
        testExceptionIncludesProblematicValue("1234567890123456789.0", BigInteger.class);
        testExceptionIncludesProblematicValue("[4, 5.5, 6]", "5.5", new TypeReference<List<Integer>>() {});
        testExceptionIncludesProblematicValue("{\"key1\": 4, \"key2\": 5.5}", "5.5", new TypeReference<Map<String, Integer>>() {});
    }

    private void testExceptionIncludesProblematicValue(String value, Class<?> valueType) {
        JsonProcessingException e = assertThrows(MismatchedInputException.class,
                () -> objectMapper.readValue(value, valueType));
        assertTrue(e.getMessage().contains("('" + value + "')"));
    }

    private void testExceptionIncludesProblematicValue(String content, String value, TypeReference<?> valueType) {
        JsonProcessingException e = assertThrows(MismatchedInputException.class,
                () -> objectMapper.readValue(content, valueType));
        assertTrue(e.getMessage().contains("('" + value + "')"));
    }
}

[cowtowncoder]

@mjustin Thank you for the test cases! These help a lot if and when tackling the issue.

[cowtowncoder]

I hope to tackle this still before 2.12.0 since helper methods were added after 2.11 and it may be relatively easy to still change them at this point.