Custom ExceptionMapper for WebApplicationException or subclasses doesn't work

[Faboli7]

Describe the bug
Hi,

According to the respective documentations, rest-client provides a default ResponseExceptionMapper that will transform any response with status >= 400 into a WebApplicationException.
Jax-rs on its side, provides a default ExceptionMapper for WebApplicationException that will convert them into the appropriate response.

This means that in a project using both, if an external service called with a rest client returns an error response, it will be mapped to a WAE that the jax-rs part of my application will remap to a Response before sending it to the client of my app. And it seems to be what happens, as my jax-rs endpoints return exactly the error response that was received by the rest client.

Now my issue is that I want to override the default ExceptionMapper for WAE. So I wrote my own, but it's never executed, even if I add @Priority(1) as advised in this issue.

Here's the code just to be sure:

@Provider
@Priority(1)
public final class WebApplicationExceptionMapper implements ExceptionMapper<WebApplicationException> {

        @Override
	public Response toResponse(WebApplicationException exception) {
		// some code to log the detail of the error before returning the status code only, 
		return Response.status(status).build();
	}    
}

I know that the default ResponseExceptionMapper from rest-client works fine because I'm able to catch the WAE in a try-catch block around the call of the client interface method.

I also have other ExceptionMappers in my project that work just fine.

I suspect a conflict with the default mapper, although @Priority is supposed to fix it.
But what is even weirder is that if I create my own exception (and implement and register the corresponding ResponseExceptionMapper in my rest clients to return it), it works as expected, but only if my exception extends RuntimeException. It doesn't work if it extends WebApplicationException.

Anyway this workaround works but it's not as elegant as I have to register this customResponseExceptionMapper in all my rest-clients with @RegisterProvider + since I can't extend WAE directly, I need to kind of reimplement its behavior myself in my custom exception. Also, I don't really like the idea of modifying the rest client part to make jax-rs work as expected.

Also worth mentioning, this happens no matter if I'm running the dev profile or any other profile.

Expected behavior
My custom ExceptionMapper should be triggered instead of the default one.

Actual behavior
My custom ExceptionMapper is ignored.

To Reproduce

Steps to reproduce the behavior:

1. implement a custom ExceptionMapper or ExceptionMapper<? extends WAE>
2. throw a WebApplicationException or the subclass
3. see that the custom mapper was not called

Environment (please complete the following information):

• Output of uname -a or ver: (but also happens in production on alpine linux)
  MINGW64_NT-10.0-19042 BEKST00080 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys
• Output of java -version:
  openjdk 11.0.7 2020-04-14 LTS
  OpenJDK Runtime Environment 20.4-(Zulu-11.39+15-win_x64)-Microsoft-Azure-restricted (build 11.0.7+10-LTS)
  OpenJDK 64-Bit Server VM 20.4-(Zulu-11.39+15-win_x64)-Microsoft-Azure-restricted (build 11.0.7+10-LTS, mixed mode)
• GraalVM version (if different from Java): not using the native build
• Quarkus version or git rev: 1.11.3
• Build tool (ie. output of mvnw --version or gradlew --version): maven 3.6.3

[gsmet]

I think the faulty behavior between the client and the server has been fixed recently with a RESTEasy upgrade. @sberyozkin would know more.

[pazkooda]

@Faboli7 Does the WAE you throw in point 2 of steps to reproduce contain response with entity/body?

[pazkooda]

Most likely duplicate of #4031

[pazkooda]

Please see if my feedback on related issue applies to your case.
If so this ticket should be closed as Works as expected.

Also to double check your case consider throwing WebApplicationException with Response but without a body - this one should be processed by ExceptionMapper - it is happening in my code.

[Faboli7]

Hi,
The response does indeed contain a body so that might be the issue.

However, I tried without a body and the ExceptionMapper is still not called, although the WebApplicationException is thrown by the client, as I can catch it in the calling resource.
Here is a reproducer.

@Path("A")
public final class ResourceA {

    @Inject
    @RestClient
    ServiceB service;

    @GET
    public Response get() {
        // a try-catch around the call catches the WAE, but the mapper is not called
        return service.get();
    }
    
}

@Path("B")
@RegisterRestClient(configKey = "B")
public interface ServiceB {
    
    @GET
    Response get();

}
(B/mp-rest/url=http://localhost:80)

@Path("B")
public final class ResourceB {

    @GET
    public Response get() {      
        return Response.status(500).build();
        // I also tried this, just to be sure, with the same result: return Response.status(500).entity(null).build();
    }
    
}

@Provider
@Priority(1)
public  class WebApplicationExceptionMapper implements ExceptionMapper<WebApplicationException> {

    @Override
    public Response toResponse(WebApplicationException exception) {
        return Response.status(400).build();
    }
    
}

Using Quarkus 1.11.3 with following extensions:

<dependency>
  <groupId>io.quarkus</groupId>
  <artifactId>quarkus-resteasy-jsonb</artifactId>
</dependency>
<dependency>
  <groupId>io.quarkus</groupId>
  <artifactId>quarkus-resteasy-multipart</artifactId>
</dependency>
<dependency>
  <groupId>io.quarkus</groupId>
  <artifactId>quarkus-rest-client-jsonb</artifactId>
</dependency>
<dependency>
  <groupId>io.quarkus</groupId>
  <artifactId>quarkus-smallrye-jwt</artifactId>
</dependency>

Something interesting though, if I change the http method of ResourceB and serviceB to HEAD, then my mapper is called and changes the status from 500 to 400 as expected. So it seems that even with no entity, a Response to a GET is considered to have a body. I also tried with OPTIONS with the same result as the GET, and originally I had the issue with a POST.

Something else, if I only change ResourceB to HEAD, but not ServiceB, to have the framework return a 405 error instead of my own code, the mapper is triggered too and changes the status from 405 to 400.

I hope this can help understand the issue better.

Anyway, the same day I created this issue, quarkus 1.12 was released and it seems like it could solve the issue, since :

MP REST Client or JAX-RS Client invocation exceptions will no longer have JAX-RS Response available by default

See migration guide paragraph

I haven't tested this version yet (I'll do that next), but it could be that the mapper is triggered now.
But anyway since my original use case was to do exactly what is now the default behavior, I should not need to overwrite the default mapper anymore.

I'll keep you posted after I try with 1.12 but it's probably still interesting to understand where the issue comes from.

[lwitkowski]

@Faboli7 your reproducer does trigger WebApplicationExceptionMapper, so it is a matter of Response having not-null entity or not - RESTeasy code shows that pretty clearly here, so I don't think there's more to understand here :)

The change introduced in 1.12 is very good, as the current behaviour - even though it is valid according to JaxRS specification - IMO breaks the principle of least astonishment.

[Faboli7]

Hi @lwitkowski,
I tested it dozens of time and it does not trigger it, except when the method is HEAD, or when resteasy returns a 405 by itself (but not with a 404) before even reaching my code.

And how could this:
return Response.status(500).entity(null).build();
be a response with a not null entity ?

I even put a try-catch around the caller and a breakpoint in the catch block to check the value of the received response entity contained in the exception and it's null, as expected.

At this point it seems that a Response created by the developer is not handled the same way as a Response created by resteasy.

EDIT: Anyway, since the behavior of all this should be different as of 1.12, maybe it's not worth wasting any more time on this.

[lwitkowski]

@Faboli7 You're right about reproducer, I used 1.12 for some reason :-/ With 1.11 it does bypass Exception Mapper, and here's the reason:

Between ResourceB and ServiceB there's whole network stack, and Resource object returned by resource is transformed into http response bytes sent through the network (possibly with content-length = 0), then received by ServiceB rest client, which creates it's own copy of Response object, with non-null entity (in your case it's an object of AbstractBuiltResponse$InputStreamWrapper class, with input stream of length zero I assume) - that's why it does not go through ExceptionMapper.

Does that make sense now?

[Faboli7]

@lwitkowski Thanks, at least I have the technical explanation now.

I'm not sure I understand why an automatic Response from resteasy (like 405) is treated differently though, as there is also network involved (ServiceB has a GET method, ResourceB exposes a HEAD method so ServiceB sends a GET request that goes over the network as well, except that there is no corresponding endpoint in ResourceB (no GET endpoint) so resteasy returns a 405, and this triggers the mapper.

Anyway it's just because I like to understand how things work. In my real project the API I call is not even written in java so I don't know what would be the behavior with an empty body (I never tested it, as the whole point of my mapper was to remove the body from the error responses of this API before forwarding them to the frontend).

So in conclusion, I don't know if this behaves as expected or not in the end, but since 1.12 handles things differently (and most importantly, the way I wanted), I don't want to bother you any longer so in my opinion we can close the issue and I'll just upgrade to 1.12 (I'm currently trying but I have issues with calls to azure on startup, seems like a conflict between netty versions in quarkus and azure. I'll open another issue if I can't manage to solve it).

Thanks all for your time.

[pazkooda]

@Faboli7: Response to HEAD by specification should not contain body (same as request). This might be why method type makes a difference.

[pazkooda]

@gsmet, @kenfinnigan: Consider closing this one - as of reporter suggestion 😃

[kenfinnigan]

Closing at reporters request