StdValueInstantiator unwraps exceptions, losing context

[milesk-amzn]

When using ObjectMapper to create objects, if the constructor throws an exception, it is "unwrapped" such that the JsonMappingException's direct cause is the innermost exception. This potentially loses important context that had been added to the exception chain by the constructor, such as which property the exception applied to.

https://github.com/FasterXML/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java#L440

I'm not entirely clear on why StdValueInstantiator is looking for an existing JsonMappingException in the exception chain rather than just unconditionally wrapping the outermost exception directly.

[cowtowncoder]

Do you have a specific example of when this logic is wrong? My experience has been that most of the time the innermost exception (root cause) is the thing that matters, and that many libraries (and JDK itself) tend to over-wrap exceptions...

I can see how this can be potentially problematic, but want to understand specific problem you encountered. And then to think if there is anything we can do to generally improve behavior. The reason I do not want to pass exception exactly as is is mostly because JDK tends to produce deep nesting of unnecessary wrappers when calling constructor via reflection. But perhaps that could be handled in bit more intelligent fashion.

[milesk-amzn]

Here's our use case where this causes problems for us:

We use Jackson to construct configuration objects from user-supplied JSON (which has already been checked to be legal JSON, and has been verified to conform to the structure of the Java objects being created). The constructors for those objects perform some validation and throw a specific exception if there are any problems. When we call readValue(), we check to see if an exception thrown is specifically a JsonMappingException with a ValidationException cause. If so, we surface the message from that exception to the user. Otherwise, we assume that an internal application failure occurred and use our fallback error handling.

Here's a simplified version of it (sorry, it's still pretty long):

import com.fasterxml.jackson.annotation.*;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.*;
import java.io.IOException;
import java.net.*;

public class Test {
    final static ObjectMapper MAPPER = new ObjectMapper()
            .configure(JsonParser.Feature.ALLOW_SINGLE_QUOTES, true);

    public static void main(String... args) {
        try {
            loadProxyConfig("{'localUri':'.','remoteUri':' bar'}");
        } catch (ValidationException e) {
            System.err.println("Couldn't load config: " + e.getMessage());
        }
    }

    static ProxyConfig loadProxyConfig(String json) throws ValidationException {
        try {
            return MAPPER.readValue(json, ProxyConfig.class);
        } catch (JsonMappingException e) {
            if (e.getCause() instanceof ValidationException) {
                throw (ValidationException) e.getCause();
            }
            // The structure has already been validated, so we don't expect
            // any other mapping exceptions
            throw new RuntimeException(e);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }

    public static class ValidationException extends Exception {
        ValidationException(String message) {
            super(message);
        }
        ValidationException(String message, Throwable cause) {
            super(message, cause);
        }
    }

    public static class ProxyConfig {
        final URI localUri;
        final URI remoteUri;

        @JsonCreator
        public ProxyConfig(@JsonProperty("localUri") String localUri,
                @JsonProperty("remoteUri") String remoteUri)
                throws ValidationException {
            this.localUri = convertUri(localUri, "localUri");
            this.remoteUri = convertUri(remoteUri, "remoteUri");
            // additional initialization occurs here that could possibly throw
            // *unanticipated* runtime exceptions, which we consider an internal
            // failure and don't want to expose directly to the user
        }

        private static URI convertUri(String uri, String propertyName)
                throws ValidationException {
            if (uri == null) {
                throw new ValidationException(propertyName + " is required");
            }
            try {
                return new URI(uri);
            } catch (URISyntaxException e) {
                // Including the cause here breaks the loadProxyConfig
                // error handling logic
                throw new ValidationException(propertyName + " is invalid: "
                        + e.getMessage(), e);
            }
        }
    }
}

So the main reason I opened this issue is because it was surprising to us that it failed in the one case where we were throwing a ValidationException with a cause. We can (and did) remove the cause as a work-around, but in general prefer as a matter of general style not to discard causes until the exception is ultimately logged or handled.

[cowtowncoder]

One note on original question: the reason for looking for an existing JsonMappingException is to reuse applicable exception instead of adding yet another layer of wrapping; as well as to preserve location information container therein (granted it would be wrapped). I have come to dislike massive onion-like layers of exception wrapping.

But I can see how unwrapping itself can be problematic. The main thing to figure out would be just whether unwrapping should be controlled globally (unwrap / dont-unwrap), or for specific case of creators.
For now, I assume that global setting would be fine for simplicity.

[cowtowncoder]

Have not had time to look further into this, but one possible work-around would be to make ValidationException extend JsonMappingException. If so, it would not be wrapped or discarded.

However: for 2.7 I think I will also change the logic of unwrapper slightly: only unwrap one level, and only if it is one of JDK's wrapper exceptions, like ExceptionInInitializerError, but NOT to unwrap "custom" exceptions. I think this makes sense as it will retain exact exception your code throws.

[cowtowncoder]

Ok I think handling is much improved now and should handle wrapping better. Basically, it only unwraps immediate "throw from static init block / constructor" error/exception, if one exists, and then either passes as is (if already JsonMappingException), or wraps as JsonMappingException with cause, copying message. I think this makes sense, and specifically preserves custom exception thrown, if any.

[alveodata]

Please i still stuck against this issue
In a spring data rest project i use a custom RuntimeException to be called in a custom Deserializer

public class LocalDateDeserializer extends StdDeserializer<LocalDate> {
 ...
    @Override
    public LocalDate deserialize(JsonParser jsonparser, DeserializationContext context) throws IOException, JsonProcessingException {
        String date = jsonparser.getText();
        String name = jsonparser.getCurrentName();
        try {
            return LocalDate.parse(date, DateTimeFormatter.ISO_LOCAL_DATE);
        } catch (DateTimeParseException e) {
            throw new ApiJacksonException("error on: " + name);
        }
    }
}

My User.class

@Data
@NoArgsConstructor
public class User extends Auditing implements Serializable {
    private static final long serialVersionUID = 1L;
 ...
    @DateTimeFormat(iso = ISO.DATE)
    @JsonFormat(pattern = "yyyy-MM-dd")
    @JsonDeserialize(using = LocalDateDeserializer.class)
    @JsonSerialize(using = LocalDateSerializer.class)
    private LocalDate birthdate;
}

When i send a POST request with a wrong date format the @ControllerAdvice catch the custom RuntimeException

But when i send a PATCH request with a wrong date format it seams that the RuntimeException is wrapped by the JsonMappingException and can't be catched by the @ControllerAdvice in the properties file i have set

spring.jackson.deserialization.wrap-exceptions = false

How to unwrap Custom RuntimeException from Json Mapping Exception
Have i missed some thing!