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

Support suppressed property when deserializing Throwable #3177

Closed
klaasdellschaft opened this issue Jun 16, 2021 · 3 comments · Fixed by #3179
Closed

Support suppressed property when deserializing Throwable #3177

klaasdellschaft opened this issue Jun 16, 2021 · 3 comments · Fixed by #3179
Milestone

Comments

@klaasdellschaft
Copy link
Contributor

Hi,

when using Jackson 2.12.x in my project to serialize / deserialize instances of Throwable, I noticed that the serialized JSON output contains the suppressed property. This property is also successfully filled in JSON if the original Throwable has suppressed exceptions. However, when deserializing the JSON back again to a cloned Throwable then this property is getting ignored, i.e. the cloned instance has an empty array of suppressed exceptions.

When debugging this issue, I think I narrowed it down to BeanDeserializerFactory.buildThrowableDeserializer(...) where the suppressed property is added to the ignorable properties.

// Java 7 also added "getSuppressed", skip if we have such data:
builder.addIgnorable("suppressed");

Based on the comment, I guess this ignoring of the suppressed property has historic reasons because it was only introduced in JDK 7, and only since jackson-databind 2.7 the Minimum JDK version is 1.7.

Given that the Minimum JDK version is even 1.8 since jackson-databind 2.12, what is your opinion on introducing deserialization support for the suppressed property? If you agree that this support should be introduced, I would give it a try to introduce it in BeanDeserializerFactory.buildThrowableDeserializer(...) and then send you a pull request. Most likely, I would implement it in a style similar to how the cause property of Throwable gets deserialized, i.e. I would add a manually constructed SettableBeanProperty that is using Throwable.addSuppressed(...) for adding the suppressed exceptions to the Throwable instance.

@cowtowncoder
Copy link
Member

Ah. Yes, your reading of code and history is spot on.

Jackson JDK baseline for building has been JDK 8 for a bit, but 2.12 is still officially JDK7 compliant for runtime.
However, 2.13 will move the baseline fully to JDK 8 so this would be possible to add -- and due to possible complications I would like the patch against 2.13 anyway.

So, PR would be welcome, and I can help with it. I think this would be a great improvement.
The way ThrowableDeserializer is implemented is not optimal, so there may be some gotchas (wrt delegation), but I think this would be doable.

klaasdellschaft added a commit to klaasdellschaft/jackson-databind that referenced this issue Jun 18, 2021
Supporting the suppressed property when deserializing Throwable
@klaasdellschaft
Copy link
Contributor Author

Hi @cowtowncoder I would be happy if you can give me feedback on the PR. The implementation is finished, and all tests are passing.

@cowtowncoder
Copy link
Member

Thanks! Will do -- will be out for a couple of days, will review once I get back!

klaasdellschaft added a commit to klaasdellschaft/jackson-databind that referenced this issue Jun 28, 2021
Addressing review comment.
Small refactoring in order to reduce code duplication.
@cowtowncoder cowtowncoder added this to the 2.13.0 milestone Jun 30, 2021
@cowtowncoder cowtowncoder changed the title Supporting the suppressed property when deserializing Throwable Support suppressed property when deserializing Throwable Jun 30, 2021
cowtowncoder added a commit that referenced this issue Jun 30, 2021
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

Successfully merging a pull request may close this issue.

2 participants