Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapDeserializer forcing JsonMappingException wrapping even if WRAP_EXCEPTIONS set to false #3068

Closed
perkss opened this issue Feb 25, 2021 · 5 comments
Milestone

Comments

@perkss
Copy link

perkss commented Feb 25, 2021

Describe the bug
I am upgrading a project from 1.9.13 to 2.12.1. The WRAP_EXCEPTION feature was added in since that as we have a custom exception now being wrapped.

The project adds a Deserializer and overrides deserialize where it throws a custom exception. We do not want this exception to be wrapped but it now gets wrapped as a JsonMappingException. Even setting WRAP_EXCEPTION to false it still wraps the exception.

The code calls wrapAndThrow and then wraps the exception ContainerDeserializerBase.

Then when handled as its already been wrapped by a JsonMappingException it is a instance of IOException so it will not unwrap the exception in the code BeanDeserializerBase.

Should we be able to get the underlying exception here or is this by design to force this wrapping?

Version information
2.12.1

To Reproduce

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;

public class JacksonIssue {

    private static final ObjectMapper OBJECT_MAPPER = getObjectMapper();

    private static ObjectMapper getObjectMapper() {
        ObjectMapper mapper = new ObjectMapper();
        mapper.configure(DeserializationFeature.WRAP_EXCEPTIONS, false);
        mapper.configure(SerializationFeature.WRAP_EXCEPTIONS, false);

        SimpleModule module = new SimpleModule("SimpleModule", new Version(1, 0, 0, ""));

        module.addDeserializer(MyContainerModel.class, new JsonDeserializer<MyContainerModel>() {
            @Override
            public MyContainerModel deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JsonProcessingException {
                throw new CustomException("This gets wrapped");
            }
        });

        mapper.addMixIn(MyJobModel.class, MyJsonJobModelMixIn.class);
        mapper.addMixIn(MyContainerModel.class, MyJsonContainerModelMixIn.class);

        mapper.registerModule(module);

        return mapper;
    }


    @Test
    void wrapExceptionIgnored() throws JsonProcessingException {
        ObjectNode jobModelJson = buildJobModelJson();
        ObjectNode containerModelJson = (ObjectNode) jobModelJson.get("containers").get("1");
        containerModelJson.remove("processor-id");

        String jsonString = new ObjectMapper().writeValueAsString(jobModelJson);

        Assertions.assertThrows(CustomException.class, () -> {
            OBJECT_MAPPER.readValue(jsonString, MyJobModel.class);
        });
    }


    private static ObjectNode buildJobModelJson() {
        ObjectMapper objectMapper = new ObjectMapper();

        ObjectNode model = objectMapper.createObjectNode();
        model.put("system", "foo");
        model.put("stream", "bar");
        model.put("partition", 1);

        ArrayNode containerModel1TaskTestSSPsJson = objectMapper.createArrayNode();
        containerModel1TaskTestSSPsJson.add(model);

        ObjectNode model2 = objectMapper.createObjectNode();
        model2.put("task-name", "test");
        model2.put("changelog-partition", 2);
        model2.put("task-mode", "Active");

        ObjectNode containerModel1TasksJson = objectMapper.createObjectNode();
        containerModel1TasksJson.put("test", model);

        ObjectNode containerModel1Json = objectMapper.createObjectNode();
        containerModel1Json.put("processor-id", "1");
        containerModel1Json.put("tasks", containerModel1TasksJson);

        ObjectNode containersJson = objectMapper.createObjectNode();
        containersJson.put("1", containerModel1Json);

        ObjectNode jobModelJson = objectMapper.createObjectNode();
        jobModelJson.put("containers", containersJson);

        return jobModelJson;
    }


    public abstract class MyJsonContainerModelMixIn {
        static final String PROCESSOR_ID_KEY = "processor-id";
        static final String CONTAINER_ID_KEY = "container-id";
        static final String TASKS_KEY = "tasks";

        public MyJsonContainerModelMixIn() {
        }

        @JsonProperty("processor-id")
        abstract String getId();

    }

    @JsonIgnoreProperties(
            ignoreUnknown = true
    )
    public abstract class MyJsonJobModelMixIn {
        @JsonCreator
        public MyJsonJobModelMixIn(@JsonProperty("containers") Map<String, MyContainerModel> containers) {
        }

        @JsonProperty("containers")
        abstract Map<String, MyContainerModel> getContainers();
    }


    public static class MyContainerModel {
        private final String id;

        public MyContainerModel(String id) {
            this.id = id;
        }

        public String getId() {
            return this.id;
        }


    }

    public static class MyJobModel {
        private final Map<String, MyContainerModel> containers;
        public int maxChangeLogStreamPartitions;

        @JsonCreator
        public MyJobModel(Map<String, MyContainerModel> containers) {
            this.containers = Collections.unmodifiableMap(containers);
            this.maxChangeLogStreamPartitions = 0;

        }


        public Map<String, MyContainerModel> getContainers() {
            return this.containers;
        }


    }


}
public class CustomException extends RuntimeException {
    private static final long serialVersionUID = 1L;

    public CustomException() {
        super();
    }

    public CustomException(String s, Throwable t) {
        super(s, t);
    }

    public CustomException(String s) {
        super(s);
    }

    public CustomException(Throwable t) {
        super(t);
    }
}

Expected behavior
<jacksonissue.CustomException> but was: <com.fasterxml.jackson.databind.JsonMappingException>

Additional context
This is due to trying to upgrade Apache Samza from a very old Jackson version where they throw an exception like the case above referenced here

@perkss perkss added the to-evaluate Issue that has been received but not yet evaluated label Feb 25, 2021
@perkss perkss changed the title MapDeserializer forcing WrapAndThrow even if WRAP_EXCEPTIONS set to false MapDeserializer forcing JsonMappingException WrapAndThrow even if WRAP_EXCEPTIONS set to false Feb 25, 2021
@cowtowncoder
Copy link
Member

cowtowncoder commented Feb 25, 2021

It does sound like a flaw: I wish I could give a definitive statement but I think that a reproduction would help answer this question. The main caveat is that only RuntimeExceptions and IOExceptions are exposed from most Jackson methods -- but I assume your custom exception is a RuntimeException.
But beyond that there may be some other edge cases wrt reflection usage. So if possible, I'd like to know the specific case(s) you have.

Assuming this is a flaw to fix, I'd have to figure out if it is small enough change it could go in 2.12.2 or 2.13.0.

@cowtowncoder cowtowncoder added 2.12 need-test-case To work on issue, a reproduction (ideally unit test) needed and removed to-evaluate Issue that has been received but not yet evaluated labels Feb 25, 2021
@perkss
Copy link
Author

perkss commented Feb 25, 2021

I have tried to create a toy example which hits WrapsAndThrows in ContainerDeserializerBase.

@cowtowncoder cowtowncoder removed the need-test-case To work on issue, a reproduction (ideally unit test) needed label Feb 26, 2021
@cowtowncoder cowtowncoder changed the title MapDeserializer forcing JsonMappingException WrapAndThrow even if WRAP_EXCEPTIONS set to false MapDeserializer forcing JsonMappingException wrapping even if WRAP_EXCEPTIONS set to false Feb 26, 2021
@cowtowncoder cowtowncoder modified the milestones: 2.10.0, 2.12.2 Feb 26, 2021
@cowtowncoder
Copy link
Member

Was straight-forward to fix and I think likelihood of regression is low (affects regular Map and EnumMap deserializers), so including it in 2.12 branch for upcoming 2.12.2 patch.

@perkss
Copy link
Author

perkss commented Feb 26, 2021

Thanks tested against the snapshot 2.12.3-SNAPSHOT and it fixed the real scenario

@cowtowncoder
Copy link
Member

@perkss great, that you for verifying it. Hoping to release 2.12.2 quite soon now, within a week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants