Skip to content

Improve handling of wrapping as DatabindException to try to preserve location info #5091

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 11, 2025
Merged

Improve handling of wrapping as DatabindException to try to preserve location info #5091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ee4360
Initial update
cowtowncoder Apr 11, 2025
377e7ba
Merge branch '3.x' into tatu/3.0/improve-exception-wrapping
cowtowncoder Apr 11, 2025
a6c52c4
Fix passing of context/parser, to preserve location
cowtowncoder Apr 11, 2025
c215b8a
Merge branch 'tatu/3.0/improve-exception-wrapping' of github.com:Fast…
cowtowncoder Apr 11, 2025
88e6fa5
...
cowtowncoder Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 53 additions & 19 deletions src/main/java/tools/jackson/databind/DatabindException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@ protected DatabindException(Closeable processor, String msg, Throwable problem)
super(processor, msg, problem);
}

protected DatabindException(Closeable processor, String msg, TokenStreamLocation loc)
{
super(msg, loc, null);
protected DatabindException(Closeable processor, String msg, TokenStreamLocation loc) {
super(processor, msg, loc, null);
}

protected DatabindException(Closeable processor, String msg, TokenStreamLocation loc,
Throwable rootCause) {
super(processor, msg, loc, rootCause);
}
protected DatabindException(String msg, TokenStreamLocation loc, Throwable rootCause) {
super(msg, loc, rootCause);
}
Expand Down Expand Up @@ -74,21 +77,62 @@ public static DatabindException from(DeserializationContext ctxt, String msg) {
return new DatabindException(_parser(ctxt), msg);
}

public static DatabindException from(DeserializationContext ctxt, String msg, Throwable problem) {
return new DatabindException(_parser(ctxt), msg, problem);
}

public static DatabindException from(SerializationContext ctxt, String msg) {
return new DatabindException(_generator(ctxt), msg);
}

public static DatabindException from(SerializationContext ctxt, String msg, Throwable problem) {
return new DatabindException(_generator(ctxt), msg, problem);
}

// // "Overrides": methods with name same as ones from JacksonException
// // (but for static methods no real overriding, static dispatch)

public static JacksonException wrapWithPath(DeserializationContext ctxt,
Throwable src, Reference ref) {
// !!! TODO: get `JsonParser`, location from `ctxt`
return JacksonException.wrapWithPath(src, ref,
(msg, cause) -> new DatabindException(msg, null, cause));
JsonParser p = _parser(ctxt);

// Copied from JacksonException.wrapWithPath()
JacksonException jme;
if (src instanceof JacksonException) {
jme = (JacksonException) src;
} else {
// [databind#2128]: try to avoid duplication
String msg = _exceptionMessage(src);
// Let's use a more meaningful placeholder if all we have is null
if (msg == null || msg.isEmpty()) {
msg = "(was "+src.getClass().getName()+")";
}
TokenStreamLocation loc = (p == null) ? null : p.currentLocation();
jme = new DatabindException(p, msg, loc, src);
}
jme.prependPath(ref);
return jme;
}

public static JacksonException wrapWithPath(SerializationContext ctxt,
Throwable src, Reference ref) {
// !!! TODO: get `JsonGenerator` from `ctxt`
return JacksonException.wrapWithPath(src, ref,
(msg, cause) -> new DatabindException(msg, null, cause));
JsonGenerator g = _generator(ctxt);

// Copied from JacksonException.wrapWithPath()
JacksonException jme;
if (src instanceof JacksonException) {
jme = (JacksonException) src;
} else {
String msg = _exceptionMessage(src);
if (msg == null || msg.isEmpty()) {
msg = "(was "+src.getClass().getName()+")";
}
// 2025-04-11, tatu: No location from generator, currently
TokenStreamLocation loc = null;
jme = new DatabindException(g, msg, loc, src);
}
jme.prependPath(ref);
return jme;
}

/*
Expand All @@ -101,16 +145,6 @@ private static JsonParser _parser(DeserializationContext ctxt) {
return (ctxt == null) ? null : ctxt.getParser();
}

public static DatabindException from(SerializationContext ctxt, String msg) {
return new DatabindException(_generator(ctxt), msg);
}

public static DatabindException from(SerializationContext ctxt, String msg, Throwable problem) {
// 17-Aug-2015, tatu: As per [databind#903] this is bit problematic as
// SerializationContext instance does not currently hold on to generator...
return new DatabindException(_generator(ctxt), msg, problem);
}

private static JsonGenerator _generator(SerializationContext ctxt) {
return (ctxt == null) ? null : ctxt.getGenerator();
}
Expand Down
28 changes: 18 additions & 10 deletions src/main/java/tools/jackson/databind/deser/SettableAnyProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package tools.jackson.databind.deser;

import java.lang.reflect.InvocationTargetException;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
Expand Down Expand Up @@ -247,7 +248,7 @@ public void set(DeserializationContext ctxt, Object instance, Object propName, O
} catch (JacksonException e) {
throw e;
} catch (Exception e) {
_throwAsIOE(e, propName, value);
_throwAsIOE(ctxt, e, propName, value);
}
}

Expand All @@ -261,31 +262,38 @@ protected abstract void _set(DeserializationContext ctxt, Object instance, Objec
*/

/**
* @param e Exception to re-throw or wrap
* @param t Exception to re-throw or wrap
* @param propName Name of property (from Json input) to set
* @param value Value of the property
*/
protected void _throwAsIOE(Exception e, Object propName, Object value)
protected void _throwAsIOE(DeserializationContext ctxt, Throwable t, Object propName, Object value)
throws JacksonException
{
if (e instanceof IllegalArgumentException) {
if (t instanceof IllegalArgumentException) {
String actType = ClassUtil.classNameOf(value);
StringBuilder msg = new StringBuilder("Problem deserializing \"any-property\" '").append(propName);
msg.append("' of class "+getClassName()+" (expected type: ").append(_type);
msg.append("; actual type: ").append(actType).append(")");
String origMsg = ClassUtil.exceptionMessage(e);
String origMsg = ClassUtil.exceptionMessage(t);
if (origMsg != null) {
msg.append(", problem: ").append(origMsg);
} else {
msg.append(" (no error message provided)");
}
throw DatabindException.from((JsonParser) null, msg.toString(), e);
throw DatabindException.from(ctxt, msg.toString(), t);
}
ClassUtil.throwIfJacksonE(e);
ClassUtil.throwIfRTE(e);
ClassUtil.throwIfJacksonE(t);
ClassUtil.throwIfRTE(t);
// let's wrap the innermost problem
Throwable t = ClassUtil.getRootCause(e);
throw DatabindException.from((JsonParser) null, ClassUtil.exceptionMessage(t), t);
// 11-Apr-2025: [databind#4603] no more general unwrapping
// Throwable t = ClassUtil.getRootCause(e);
// ... just one:
if (t instanceof InvocationTargetException ite) {
t = ite.getTargetException();
ClassUtil.throwIfRTE(t);
ClassUtil.throwIfJacksonE(t);
}
throw DatabindException.from(ctxt, ClassUtil.exceptionMessage(t), t);
}

private String getClassName() { return ClassUtil.nameOf(_setter.getDeclaringClass()); }
Expand Down
23 changes: 16 additions & 7 deletions src/test/java/tools/jackson/databind/exc/UnwrapRootCause4603Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.Test;

import tools.jackson.core.JacksonException;
import tools.jackson.core.TokenStreamLocation;
import tools.jackson.databind.*;
import tools.jackson.databind.testutil.DatabindTestUtil;

Expand Down Expand Up @@ -56,25 +57,28 @@ public String getQux() {
}
}

private static final String JSON = a2q("{'value':3}");

private final ObjectMapper MAPPER = newJsonMapper();

// Whether disabled or enabled, should get ArithmeticException
@Test
public void testExceptionWrappingConfiguration()
throws Exception
{
JacksonException disabledResult = _tryDeserializeWith(MAPPER);
assertInstanceOf(DatabindException.class, disabledResult);
JacksonException result = _tryDeserializeWith(MAPPER);
assertInstanceOf(DatabindException.class, result);
// We are throwing exception inside a setter, but InvocationTargetException
// will still be unwrapped
assertInstanceOf(CustomException.class, disabledResult.getCause());
assertInstanceOf(CustomException.class, result.getCause());
TokenStreamLocation loc = result.getLocation();
assertNotSame(TokenStreamLocation.NA, loc);
// happens to point to location after `3`
assertEquals(1, loc.getLineNr());
assertEquals(11, loc.getColumnNr());
}

private JacksonException _tryDeserializeWith(ObjectMapper mapper) {
return assertThrows(JacksonException.class,
() -> mapper.readValue(JSON, Feature1347DeserBean.class)
() -> mapper.readValue(a2q("{'value':3}"), Feature1347DeserBean.class)
);
}

Expand All @@ -83,7 +87,12 @@ public void testWithAnySetter()
throws Exception
{
DatabindException result = assertThrows(DatabindException.class,
() -> MAPPER.readValue(a2q("{'a':3}"), AnySetterBean.class));
() -> MAPPER.readValue(a2q("{'a':72}"), AnySetterBean.class));
assertInstanceOf(CustomException.class, result.getCause());
TokenStreamLocation loc = result.getLocation();
assertNotSame(TokenStreamLocation.NA, loc);
// happens to point to location after `72`
assertEquals(1, loc.getLineNr());
assertEquals(8, loc.getColumnNr());
}
}