rmannibucau commented on code in PR #91:
URL: https://github.com/apache/johnzon/pull/91#discussion_r865574510


##########
johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MappingParserImpl.java:
##########
@@ -399,36 +403,43 @@ private Object buildObject(final Type inType, final 
JsonObject object, final boo
             final JsonValue jsonValue = jsonEntry.getValue();
             final JsonValue.ValueType valueType = jsonValue != null ? 
jsonValue.getValueType() : null;
 
-            if (JsonValue.class == value.paramType) {
-                value.writer.write(t, jsonValue);
-                continue;
-            }
-            if (jsonValue == null) {
-                continue;
-            }
+            try {
+                if (JsonValue.class == value.paramType) {
+                    value.writer.write(t, jsonValue);
+                    continue;
+                }
+                if (jsonValue == null) {
+                    continue;
+                }
 
-            final AccessMode.Writer setterMethod = value.writer;
-            if (NULL == valueType) { // forced
-                setterMethod.write(t, null);
-            } else {
-                Object existingInstance = null;
-                if (config.isReadAttributeBeforeWrite()) {
-                    final Mappings.Getter getter = 
classMapping.getters.get(jsonEntry.getKey());
-                    if (getter != null) {
-                        try {
-                            existingInstance = getter.reader.read(t);
-                        } catch (final RuntimeException re) {
-                            // backward compatibility
+                final AccessMode.Writer setterMethod = value.writer;
+                if (NULL == valueType) { // forced
+                    setterMethod.write(t, null);
+                } else {
+                    Object existingInstance = null;
+                    if (config.isReadAttributeBeforeWrite()) {
+                        final Mappings.Getter getter = 
classMapping.getters.get(jsonEntry.getKey());
+                        if (getter != null) {
+                            try {
+                                existingInstance = getter.reader.read(t);
+                            } catch (final RuntimeException re) {
+                                // backward compatibility
+                            }
                         }
                     }
+                    final Object convertedValue = toValue(
+                            existingInstance, jsonValue, value.converter, 
value.itemConverter,
+                            value.paramType, value.objectConverter,
+                            isDedup() ? new JsonPointerTracker(jsonPointer, 
jsonEntry.getKey()) : null, inType);
+                    if (convertedValue != null) {
+                        setterMethod.write(t, convertedValue);
+                    }
                 }
-                final Object convertedValue = toValue(
-                        existingInstance, jsonValue, value.converter, 
value.itemConverter,
-                        value.paramType, value.objectConverter,
-                        isDedup() ? new JsonPointerTracker(jsonPointer, 
jsonEntry.getKey()) : null, inType);
-                if (convertedValue != null) {
-                    setterMethod.write(t, convertedValue);
-                }
+            } catch (SetterMappingException alreadyHandled) {
+                throw alreadyHandled;
+            } catch (Exception e) {

Review Comment:
   Was thinking the same about the versioning but the impact is that we should 
probably do it for all exceptions to not look half baked - this is why i 
proposed to stick to a backward compat option and refine the try/catch in 
nested calls and not around the whole call  but Im happy with the 1.3 one if 
coverage is reached too.
   
   To refine my qtatement, issue is not having directly or not the exception 
but more about not rewrapping it in a minor version. Concretely if today i know 
my cause is the 3rd cause then it must not become the 4th in 1.2.19 - ok in 
1.3. similarly for the type - cause due to jsonb abstraction, to stay portable, 
classname is sometimes checked.
   
   Still thinking out loud but think we should just lean toward 1.3 here to 
ease things for everyones, wdyt?
   



-- 
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: dev-unsubscr...@johnzon.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to