FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.52k stars 1.38k forks source link

Keep stacktrace when re-throwing exception with `JsonMappingException` #4603

Open pn-santos opened 4 months ago

pn-santos commented 4 months ago

Is your feature request related to a problem? Please describe.

I have code that re-throws an exception wrapped in a custom exception during de-serialization via builder setters, e.g.

@JsonProperty
Builder property(String value) {
    try {
        (...)
    } catch (Exception ex) {
        throw MyCustomException(..., ex);
    }

When that happens Jackson will unconditionally wrap the root cause with a JsonMappingException in:

protected IOException _throwAsIOE(JsonParser p, Exception e) throws IOException
{
    ClassUtil.throwIfIOE(e);
    ClassUtil.throwIfRTE(e);
    // let's wrap the innermost problem
    Throwable th = ClassUtil.getRootCause(e);
    throw JsonMappingException.from(p, ClassUtil.exceptionMessage(th), th);
}

The custom exception is thus "lost" from the upstream perspective (i.e. code that triggered de-serialization).

This makes it impossible to propagate the custom exception with a cause upstream (it can be propagated but it cannot have a cause set).

Describe the solution you'd like

Be able to control whether Jackson will wrap the root cause or the outer most exception via object mapper config.

Usage example

No response

Additional context

In my particular use case, the idea is that if an exception is thrown by the application, if the custom exception is found in the causal chain, the handling of the exception is different than if it's not. But it's still useful to have the complete stacktrace for logging/debugging purposes.

pjfanning commented 4 months ago

I doubt whether this will be changed in Jackson 2.x (if at all).

You can catch exceptions and look at the cause you yourself.

} catch (Exception e) {
  if (e.getCause() instanceof SomeException) {
    // do something
  } else {
    // do something else
  }
}

Even if was to be changed, you would need to wait weeks or months for the next release.

My recommendation is to code your own solution that gets the cause from the Exception. You may need to even get the cause of the cause (i.e walk the graph).

pn-santos commented 4 months ago

Yeah That is what I'm doing now, I though it would be worth a shot to report it since I didn't find it in existing or closed issues.

The main reason I think it would be useful is that it would allow handling it in a "central" place instead of on every POJO de-serializer.

I completely understand if this goes into the "won't do cause not sufficiently important" bucket 👍🏼

pjfanning commented 4 months ago

You've got to understand that many existing developers will have exception handling that works with how Jackson has long worked. If we change it to suit you then we break the code for lots of other users. If we were to do anything it would be need to configurable and disabled by default. My vote is to not do anything.

cowtowncoder commented 4 months ago

Quick note before reading discussion in detail: there are these features you can try:

which exist and handle some aspects. Probably not quite what you ask but somewhat related.

@pn-santos Fwtw, I would be open to having another (set of) feature(s) for, like, UNWRAP_ROOT_CAUSE or such (with default setting to work like things currently do), if you wanted to try a PR.

pn-santos commented 4 months ago

Quick note before reading discussion in detail: there are these features you can try

I already have the WRAP_EXCEPTIONS disabled but it doesn't affect the root cause behaviour I mentioned

I would be open to having another (set of) feature(s) for, like, UNWRAP_ROOT_CAUSE or such (with default setting to work like things currently do), if you wanted to try a PR.

👍🏼 yeah that was what I had in mind, I'll try and see if I can find the time to give a PR a shot.