quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.53k stars 2.61k forks source link

Customize list of exceptions to unwrap in resteasy-reactive #42089

Closed kevinferrare closed 1 month ago

kevinferrare commented 1 month ago

Description

Exceptions handling declared via ServerExceptionMapper annotation are not called when the exception is wrapped into another exception.

Looking at quarkus code, it looks like the RuntimeExceptionMapper class is responsible for unwrapping exceptions, but it only unwraps exceptions that are in a predefined list computed at build time via build items.

Example for resteasy reactive extension: https://github.com/quarkusio/quarkus/blob/6619648e82b15b55038bd9686f0ee71d4fcdf9e9/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveScanningProcessor.java#L122

I propose to add a configuration property to allow users to customize this list of exceptions to unwrap.

Implementation ideas

No response

quarkus-bot[bot] commented 1 month ago

/cc @FroMage (resteasy-reactive), @geoand (resteasy-reactive), @stuartwdouglas (resteasy-reactive)

geoand commented 1 month ago

I think this makes sense

gsmet commented 1 month ago

Is it a custom exception that you want to unwrap? Or something that would make sense in general?

geoand commented 1 month ago

It makes sense to have this be configurable. I planned to do it when someone asked 😀

gsmet commented 1 month ago

Oh yeah, I totally agree but I wanted to know the use case as it might be something we want to add as default anyway for better user experience.

geoand commented 1 month ago

One question I do have however is whether the feature should be enabled by a configuration property or some new annotation.

The reason the configuration property (although totally reasonable) can be suboptimal is that the IDE doesn't provide any assistance or validation of the value

geoand commented 1 month ago

42094 is what I have in mind

kevinferrare commented 1 month ago

Thank you for the quick and welcoming feedback! @gsmet

Is it a custom exception that you want to unwrap? Or something that would make sense in general? It is both, we have java.util.concurrent.CompletionException and a custom exception, thrown by an internal library.

@geoand After opening the issue, I started to wonder what would be the drawback of also optionally providing the option of just unwrapping all exceptions unconditionally until a handler is found.

In our use case, I don't think this would lead to unwanted behaviours.

geoand commented 1 month ago

After opening the issue, I started to wonder what would be the drawback of also optionally providing the option of just unwrapping all exceptions unconditionally until a handler is found.

Although this might make sense, it is however not how the JAX-RS / Jakarta REST specifies things, so it's best not to make this the default behavior.