Allow for Global Override of Extension Specific Exception Mappers

[The-Funk]

Description

Certain extensions may contain their own ExceptionMapper implementations. For instance resteasy-reactive-jackson contains the following mapper for MismatchedInputExceptions.

Some of my users have grown accustomed to a standardized payload when an exception is thrown from some of my RESTful services. For the most part I map exceptions using a universal exception mapper that handles Throwable.

In instances where a standardized exception response payload is requested it would be nice to be able to override provided mappers with more generic application specific mappers.

Implementation ideas

Exception mapping priority could be calculated differently such that higher priority mappers take precedence over exact class matches. In this way a more generic Throwable mapper could take precedence over a more specific MismatchedInputMapper.

This could be a feature flag: (Global config) quarkus.exceptions.mapping-strategy= one of (PRIORITY_FIRST, CLASS_HIERARCHY_FIRST)

And this feature could be domain specific as well: (Domain specific) quarkus.exceptions."com.example".mapping-strategy= one of (PRIORITY_FIRST, CLASS_HIERARCHY_FIRST)

[quarkus-bot[bot]]

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

[geoand]

I totally agree this is a problem but I have not come up with any good ideas on how to address it without causing major headaches to people that expect the standard behavior.

@FroMage any ideas?

[The-Funk]

@geoand I've run into another bit of weirdness that I think this would help with. In our different services a lot of my devs have copied a global exception mapper we created, and if the mapper exists in the dev's project, this "just works". In short the devs have an exception mapper that implements ExceptionMapper<Throwable> and as long as that exists in the Quarkus application code, everything is hokey pokey.

A problem arises when this code is then moved into an extension. I was reading about that here.

Things have changed a bit since 2020 but what I ended up doing is creating a Dynamic Feature and putting the filter inside the dynamic feature as a static class.

 @Provider
 public class GenericExceptionHandlerFeature implements DynamicFeature {

    @Override
    public void configure(ResourceInfo resourceInfo, FeatureContext context) {
        context.register(GenericExceptionHandler.class);
    }

    public static class GenericExceptionHandler implements ExceptionMapper<Throwable> {

        private static final Logger logger = Logger.getLogger(GenericExceptionHandler.class);

        @Override
        public Response toResponse(Throwable ex) {
            return mapExceptionToResponse(ex);
        }

        private Response mapExceptionToResponse(Throwable ex) {
            // Override w/ other custom Error
            if (ex instanceof WebApplicationException){
                Response intercepted = ((WebApplicationException) ex).getResponse();
                JsonObject error = new JsonObject()
                        .put("status", intercepted.getStatus())
                        .put("title", ex.getMessage() != null ? ex.getMessage() : "unknown error has occurred");
                logger.warn("Generic WebApplicationException caught: " + (ex.getMessage() != null ? ex.getMessage() : "no message/cause provided"));
                logger.debug("StackTrace: ",ex);
                return Response.fromResponse(intercepted)
                        .entity(error.toString())
                        .build();
            }
            // Special mappings
            else if (ex instanceof IllegalArgumentException) {
                logger.error("Illegal argument: ", ex);
                return Response.status(400).entity(ex.getMessage()).build();
            }
            // Use 500 (Internal Server Error) for all other unassociated exceptions
            else {
                logger.error("Failed to process request due to error: ", ex);
                return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
            }
        }
    }
}

Then I registered the class as an AdditionalBeanBuildItem.

@BuildStep
AdditionalBeanBuildItem buildGenericExceptionHandlerFeature() {
    return AdditionalBeanBuildItem.unremovableOf(GenericExceptionHandlerFeature.GenericExceptionHandler.class);
}

This doesn't appear to be working as expected, however my request/response filters are working just fine following this exact same formula, so I have to wonder if moving the mapper to the extension somehow changed its priority.

[geoand]

@The-Funk registration of exception mappers for extensions can be done in multiple ways.

The easiest would be to add a META-INF/beans.xml file in src/main/resources of the runtime module. That would force Quarkus to search the entire extension for CDI, JAX-RS and other annotations. If you want to go down the build item route, you can use io.quarkus.resteasy.reactive.spi.ExceptionMapperBuildItem

[The-Funk]

@geoand would jandex work instead of the beans.xml? I have the jandex plugin in my runtime with the make-index goal but for whatever reason the ExceptionMapper doesn't work. Might be worthwhile to go the build item direction since it's explicit.

[geoand]

If it's properly configured, yes, it should work

[FroMage]

I'm not too sure about having extension mappers for Throwable get a higher priority than more specific ones such as, say AuthenticationException or other framework-provided exceptions for which a special mapper exists that you may want to keep.

The problem is that once you declare a mapper for Throwable that has a higher priority, you can't delegate back to better finer-grained specialised exception mappers. So you'll have to either re-implement them in your mapper, or find a way to delegate to them, which would be returning null if you were using @ServerExceptionMapper which you're not using in your example, and using ExceptionMapper there's no way to "bail out", and your example doesn't bail out: it maps anything into a 500, which is guaranteed to be wrong for some exceptions.

Reading your initial issue, you list a specific exception mapper being problematic for you. Could it be that you know of certain problematic mappers and then you'd like your exception mapper to apply instead of those mappers using a higher priority? Because I think we can already do that, using @ServerExceptionMapper with more than one exception type: https://javadoc.io/doc/io.quarkus.resteasy.reactive/resteasy-reactive/latest/org/jboss/resteasy/reactive/server/ServerExceptionMapper.html#value()

[The-Funk]

@FroMage I think @ServerExceptionMapper should provide enough flexibility for my use case. This would be an easy way for me to ensure consistency in error reporting to downstream consumers while not changing the expected priority of global exception mappers. Now that I understand the criteria by which exception mappers are evaluated/ordered, what you suggest makes a lot of sense.

[allertonm]

I just wanted to chime in and up-vote this general issue. We got caught by the same mapper, whose behaviour is IMO incorrect for many cases - but there is a general issue with the JAX-RS mechanism of using the most specific type, when as a developer you have very little visibility of what mappers exist for what types, and what their behaviour is.

[mrickly]

We have this problem too. It would be nice to have a way to pretend that some specific ExceptionMapper are not there (black list), or even better to white list the ExceptionMappers we want. I don't know if its feasible, but then the JAKARTA REST priority rules would simply apply to the remaining ExceptionMappers.

[geoand]

That's interesting.

How do you envision you would blacklist or whitelist exception mappers?

[mrickly]

It seems that this type of idea (tailoring the build jar by removing classes specified via properties) already exists in Quarkus: https://quarkus.io/guides/class-loading-reference#hidingremoving-classes-and-resources-from-dependencies Maybe the general mechanism could be modified/extended for this particular use case (?)

[geoand]

It seems that this type of idea (tailoring the build jar by removing classes specified via properties) already exists in Quarkus: https://quarkus.io/guides/class-loading-reference#hidingremoving-classes-and-resources-from-dependencies

Exactly.

Maybe the general mechanism could be modified/extended for this particular use case (?)

Have you tried it to see that it works?

[mrickly]

The very naive approach of trying to exclude DefaultMismatchedInputException for instance using

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

runs into

[ERROR]   EchoTestResourceIT.delete » Runtime java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
    [error]: Build step io.quarkus.resteasy.reactive.jackson.deployment.processor.ResteasyReactiveJacksonProcessor#exceptionMappers threw an exception: java.lang.NoClassDefFoundError: io/quarkus/resteasy/reactive/jackson/runtime/mappers/DefaultMismatchedInputException
    at io.quarkus.resteasy.reactive.jackson.deployment.processor.ResteasyReactiveJacksonProcessor.exceptionMappers(ResteasyReactiveJacksonProcessor.java:135)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:849)
    at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
    at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
    at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2516)
    at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2495)
    at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1521)
    at java.base/java.lang.Thread.run(Thread.java:1583)
    at org.jboss.threads.JBossThread.run(JBossThread.java:483)

[geoand]

We can easily fix that, but this makes me wonder whether we should have a Quarkus REST specific mechanism for ignoring mappers...

[FroMage]

@mrickly so, let's say you could disable that MismatchInputExceptionMapper, which exception mapper would you like to handle the exception, then?

[mrickly]

@FroMage We have two rather general ExceptionMapper (ExceptionMapper<Exception> and ExceptionMapper<ValidationException>) which use a delegation pattern (depending on the exception subtype). The response is based on application/problem+json as specified in https://www.rfc-editor.org/rfc/rfc7807 Whenever there is an ExceptionMapper with a better match than ours in the classpath, that mapper will be used instead by virtue of the Jakarta rule set.

[FroMage]

Would a workaround like this help?

@ServerExceptionMapper
public Response toProblemJson(Exception x) {
  // ...
}
@ServerExceptionMapper
public Response toProblemJson(MismatchedInputException x) {
  return toProblemJson((Exception)x);
}

This should disable the problematic more-specific mapper in favour of yours.

[mrickly]

Your workaround looks nice and lightweight. The drawback compared to a whitelisting of desired ExceptionMappers is that it has to be done for every ExceptionMapper that is visible to the Jakarta resolver, and we don't control that.

[FroMage]

For both white/black lists you don't control the set of exception mappers provided by Quarkus or extensions.

A blacklist is probably going to be shorter than a whitelist, though.

[mrickly]

Not sure about that. Our whitelist would consist of our two ExceptionMapper . The blacklist would have to exclude every Exception Mapper which happens to be provided by the libraries and is similar in spirit to your proposal (of overriding them).

[FroMage]

A whitelist of two exception mappers would likely break any extension that defines and rely on their own exception mappers.

I'm thinking (without any data to back it up, just opinion) that there are more required exception mappers than exception mappers that happen to break your application.

[mrickly]

I think for now I'm going to limit myself to writing our own MismatchedInputExceptionMapper, until some users complain about other Exceptions that are not treated to their liking.