Jakarta - EE 10 - SmallRye Metrics test failure

[gsmet]

Since the upgrade to RESTEasy 6.1, we have a failure in the following SmallRye Metrics test in the jakarta-rewrite branch (for more information, see https://github.com/quarkusio/quarkus/tree/main/jakarta#build-locally):

[ERROR] io.quarkus.smallrye.metrics.jaxrs.JaxRsMetricsTestCase.testMethodThrowingException  Time elapsed: 0.264 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:166)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:161)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:628)
	at io.quarkus.smallrye.metrics.jaxrs.JaxRsMetricsTestCase.testMethodThrowingException(JaxRsMetricsTestCase.java:71)

My guess is that a behavior has changed in RESTEasy but I want us to get more details about what is exactly going on before involving James Perkins.

[quarkus-bot]

/cc @Ladicek, @ebullient, @jmartisk, @phillip-kruger, @radcortez

[gsmet]

@jmartisk could you have a look at this failure? It's working with current RESTEasy and IIRC it was working with RESTEasy 6.0 too. So my guess is that there's a change of behavior in RESTEasy 6.1 that we need to spot.

[jmartisk]

I'll double check it, but my suspicion is that it might have been a change where a ContainerResponseFilter is now called for responses that had an unmapped exception, which didn't happen before?! This looks suspicious: https://issues.redhat.com/browse/RESTEASY-3097

[gsmet]

@jmartisk btw, #27807 might be related to this one.

[jmartisk]

Yeah so the thing is that there is now a default ExceptionMapper, so all processing exceptions get mapped, and a ContainerResponseFilter is now called for every response. Before, it was only called
for requests that were either success or had a mapped exception. Thus requests that used to be detected as unmapped errors (because the filter wasn't called for them) are now detected as successful, and the unexpected metric (for successful requests) is incremented.

The question is how should we detect an "unmapped" exception now that basically all exceptions to through some ExceptionMapper.
I would say that as unmapped, we should still consider those that were handled by the default exception mapper, which maps the response code to 500. But it doesn't seem like the public JAX-RS API allows to detect
what ExceptionMapper was used, only inspect the resulting response.

So perhaps we could go with a solution that assumes that custom exception mappers will map the status code to something other than 500, and count 500 towards "unsuccessful" and all other codes towards "successful".
This fixes the failing test as well as passes the MP Metrics TCK.

We could also unify the behavior with what we have with RESTEasy Reactive, because there we currently count ALL exceptions towards "unsuccessful" requests, which sounds wrong.
I'll send a PR in a bit