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