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

Keep stacktrace when re-throwing exception with JsonMappingException #4603

Open
pn-santos opened this issue Jun 27, 2024 · 5 comments
Open

Comments

@pn-santos
Copy link

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.

@pn-santos pn-santos added the to-evaluate Issue that has been received but not yet evaluated label Jun 27, 2024
@pjfanning
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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

  • DeserializationFeature.WRAP_EXCEPTIONS (default: true)
  • SerializationFeature.WRAP_EXCEPTIONS (default: true)

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
Copy link
Author

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.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Jul 4, 2024
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

3 participants