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

Add support for custom Jackson JsonMappingException exception mapper #36155

Closed
danifalconr24 opened this issue Sep 26, 2023 · 15 comments
Closed
Labels
area/jackson Issues related to Jackson (JSON library) kind/bug Something isn't working triage/needs-reproducer We are waiting for a reproducer.

Comments

@danifalconr24
Copy link

danifalconr24 commented Sep 26, 2023

Describe the bug

Using Quarkus 2.16.10.Final

Hello,

The issue im having is that im not able to provide custom exception mappers for exceptions like ValueInstantiationException or even better would be being able to provide a mapper for JsonMappingException which is a more generic type, but, this mappers are getting ignored, because in the case of ValueInstantiationException the exception is getting mapped to a WebApplicationException by the resteasy reactive jackson quarkus extension, another example could be the InvalidFormatException which is not getting mapped into a WebApplicationException but the only way to override the default exception mapper is to provide an specific mapper for this exception, it could be much better if we can just create a mapper for JsonMappingException and handle all this Jackson exceptions directly there because they all extend from this exception.

Expected behavior

Support the possibility to provide a JsonMappingException and get all this exceptions that extends from this one

Actual behavior

Handle every exception differently, anď map them to WebApplicationException.

How to Reproduce?

Reproduce:

ValueInstantiationException

1 -> have some endpoint that receives an object with any enum property
2 -> request with invalid enum value
3 -> ValueInstantiationException is mapped somewhere to a WebApplicationException

InvalidFormatException

1 -> have some endpoint that receives an object with an Integer property
2 -> request with a string that cannot be mapped to an Integer. ie: "abcd"
3 -> We cannot handle this exception inside the JsonMappingException mapper because there is a more specific exception mapper provided by Jackson library

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@danifalconr24 danifalconr24 added the kind/bug Something isn't working label Sep 26, 2023
@quarkus-bot quarkus-bot bot added the area/jackson Issues related to Jackson (JSON library) label Sep 26, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 26, 2023

/cc @geoand (jackson), @gsmet (jackson)

@geoand
Copy link
Contributor

geoand commented Sep 27, 2023

Thanks a lot for reporting the issue.

Would you mind attaching a sample application that behaves as you have described?

Asking because I would like to see how this is different from what #29661 addressed.

@geoand geoand added the triage/needs-reproducer We are waiting for a reproducer. label Sep 27, 2023
@geoand
Copy link
Contributor

geoand commented Oct 13, 2023

Closing as we never got feedback

@geoand geoand closed this as not planned Won't fix, can't repro, duplicate, stale Oct 13, 2023
@krismatusiak
Copy link

Hey @geoand @gsmet Could we please reopen this issue?

I have an app sample here that shows the problem:
https://github.com/krismatusiak/quarkus-exception-handling

@geoand
Copy link
Contributor

geoand commented Sep 6, 2024

@krismatusiak looking at your sample I am not sure I understand the problem

@krismatusiak
Copy link

@geoand Thanks for looking so fast ;) I appreciate that :)

The challenge is to make all the tests pass ;)
Right now the JsonMappingException cannot be caught in the ConfusedExceptionHandler.
So even though I am setting the status code to 200 there (for whatever reason), this is not working, because the WebApplicationException consumed the JsonMappingException ;) Very dramatic.
On the other hand, catching WebApplicationException is also not productive and leads to other issues.

So the problem is this: I want to customize the status code (and such) when the JsonMappingException is thrown, and I cannot do that now.

You provided a good solution for a similar case of the MismatchedInputException: this can be caught in the hanlders (= yeey).
But so the JsonMappingException is actually a parent of MismatchedInputException, so: not all that different. Perhaps the parent is the better target for allowing customizations of the response?

@geoand
Copy link
Contributor

geoand commented Sep 6, 2024

Thanks for explanations!

So IIUC, you are seeing what is described in #41479.

Can you try adding:

quarkus.class-loading.removed-resources."io.quarkus\:quarkus-rest-jackson"=io/quarkus/resteasy/reactive/jackson/runtime/mappers/DefaultMismatchedInputException.class

What that does is to disable the Quarkus' built-in exception mapper for MismatchedInputException

@krismatusiak
Copy link

Hey @geoand,

Yes indeed, this is just like #41479
I read the conversation there, and I follow the argument that Quarkus 'ships' with its own mapper.
This, to me signals a conscious decision and not an accidental bug :)

That said, the customization of the response code should be made to be easy, regardless.
And by 'easy' I understand setting a boolean in the config :) Well, perhaps providing a list of exceptions that I would like to handle myself (is that possible? I think I came across a conversation about this somewhere, but cannot find it anymore), would also be 'easy'. And by 'easy' I also mean 'easy for ME, the developer' :) I totally understand that 'my easy' is not 'your easy' :)

And now for the adding of quarkus.class-loading.removed-resources."io.quarkus:quarkus-rest-jackson"=io/quarkus/resteasy/reactive/jackson/runtime/mappers/DefaultMismatchedInputException.class

So: IntelliJ does not like. It keeps reformatting this: cutting the 'key' off and adding more and more equal signs in the 'value', for some reason, in its configuration UI. So that does not fly there, regardless, even if its something werid on Intellij env var UI side. (vide screenshot one)

Screenshot 2024-09-09 at 09 21 47

In the commandline, however, I do get to pass it along. And so there: not only does the one failing test not pass, but also other tests are failing now. It is possible that I am not passing the var correctly or Im making a typo of some kind? I cannot see it. (vide screenshot two)

Screenshot 2024-09-09 at 09 09 18

@geoand
Copy link
Contributor

geoand commented Sep 9, 2024

#43126 should make things better (I used a sample application to verify)

@krismatusiak
Copy link

krismatusiak commented Sep 9, 2024

@geoand

For me, and the app that I provided, it (#43126 ) does not work (yet?).
Oh, but so it is not merged yet?
Im confused ;) Did we expect it to work for my app?

@geoand
Copy link
Contributor

geoand commented Sep 9, 2024

No, unfortunately this won't until #43126 is merged.

@krismatusiak
Copy link

krismatusiak commented Sep 9, 2024

Ok, got it.

Would you consider simplifying this (perhaps in addition to this property) into an easily-named boolean flag?

disableNativeMismatchedInputExceptionHandler: true

or some kind of list:
disableHandlers: - MismatchedInputException

@geoand
Copy link
Contributor

geoand commented Sep 9, 2024

Yeah, that's definitely up for discussion :)

@krismatusiak
Copy link

Lets discuss ;)

What would make most sense for everyone and not add too much complexity?

@geoand
Copy link
Contributor

geoand commented Sep 9, 2024

I'm on the fence. WDYT @gsmet ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/jackson Issues related to Jackson (JSON library) kind/bug Something isn't working triage/needs-reproducer We are waiting for a reproducer.
Projects
None yet
Development

No branches or pull requests

3 participants