Rest-Client-Reactive: Custom WebApplicationExceptions not accessable by MP-Fault-Tolerance

[fwippe]

Describe the bug

When using the retry mechanism of MP-Fault-Tolerance you usually do not want to retry on Status 400, i.e. bad requests. To achieve that, I annotated a method with @Retry(abortOn = BadRequestException.class) and converted 400 responses to a BadRequestException in a custom ResponseExceptionMapper<WebApplicationException>. That used to work with the old "Resteasy client". Unfortunately, with Rest-Client-Reactive all WebApplicationExceptions are wrapped by corresponding ClientWebApplicationExceptions, so that the Retry is not able to distinguish between response codes.

See #22777 for another issue regarding exception wrapping.

Expected behavior

Custom WebApplicationException types are passed through for interceptors like Retry

Actual behavior

All WebApplicationExceptions are converted to ClientWebApplicationExceptions

How to Reproduce?

@RegisterRestClient
@RegisterProvider(RestView.BadRequestExceptionMapper.class)
public interface RestView {
    @POST
    @Path("/test")
    @Retry(abortOn = BadRequestException.class)
    String test(String param);

    class BadRequestExceptionMapper implements ResponseExceptionMapper<WebApplicationException> {
        @Override
        public WebApplicationException toThrowable(Response response) {
            return new BadRequestException();
        }

        @Override
        public boolean handles(int status, MultivaluedMap<String, Object> headers) {
            return status == HttpStatus.SC_BAD_REQUEST;
        }
    }
}

Output of uname -a or ver

Microsoft Windows [Version 10.0.19042.1415]

Output of java -version

openjdk version "11.0.3" 2019-04-16 OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.3+7) OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.3+7, mixed mode)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.6.1.Final

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

Apache Maven 3.8.2 (ea98e05a04480131370aa0c110b8c54cf726c06f)

Additional information

No response

[quarkus-bot]

/cc @Ladicek, @michalszynkiewicz

[Ladicek]

If smallrye/smallrye-fault-tolerance#524 was implemented, that would hopefully work. I'll bump that on my priority list.

[fwippe]

Any chance of bumping SmallRye Fault-Tolerance to 5.2.2 for Quarkus 2.7 to use that feature?

[Ladicek]

There's no release yet. There will be one soon, but it won't make it into 2.7.

[Ladicek]

This is fixed in SmallRye Fault Tolerance 5.3.0, where in the non-compatible mode (which is on by default in Quarkus), exception cause chains are inspected.

[AndreasPetersen]

This does not seem to work with Quarkus 2.15.3. Please see this reproducer: https://github.com/AndreasPetersen/quarkus-smallrye-fault-tolerance-exception-cause-chains.

The REST client is annotated with @Retry(abortOn = NotFoundException.class), with an exception mapper producing such an exception. However, the retry does not abort, as evident by the failing test where the endpoint is called 4 times instead of 1. From the log we can see that NotFoundException.class is part of the cause chain:

2023-01-23 15:43:48,608 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-0) HTTP Request to /hello failed, error id: efe2e439-358a-4f61-8704-a76c637e92fe-1: org.jboss.resteasy.reactive.ClientWebApplicationException: Received: 'HTTP 404 Not Found' when invoking: Rest Client method: 'org.acme.NotFoundRestClient#notFound'
	at org.jboss.resteasy.reactive.client.impl.RestClientRequestContext.unwrapException(RestClientRequestContext.java:173)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.handleException(AbstractResteasyReactiveContext.java:322)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:173)
	at org.jboss.resteasy.reactive.client.impl.RestClientRequestContext$1.lambda$execute$0(RestClientRequestContext.java:279)
	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:264)
	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:246)
	at io.vertx.core.impl.EventLoopContext.lambda$runOnContext$0(EventLoopContext.java:43)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:569)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: javax.ws.rs.NotFoundException: HTTP 404 Not Found
	at org.acme.NotFoundExceptionMapper.toThrowable(NotFoundExceptionMapper.java:12)
	at org.acme.NotFoundExceptionMapper.toThrowable(NotFoundExceptionMapper.java:9)
	at io.quarkus.rest.client.reactive.runtime.MicroProfileRestClientResponseFilter.filter(MicroProfileRestClientResponseFilter.java:36)
	at org.jboss.resteasy.reactive.client.handlers.ClientResponseFilterRestHandler.handle(ClientResponseFilterRestHandler.java:21)
	at org.jboss.resteasy.reactive.client.handlers.ClientResponseFilterRestHandler.handle(ClientResponseFilterRestHandler.java:10)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.invokeHandler(AbstractResteasyReactiveContext.java:229)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:145)
	... 12 more

[Ladicek]

Sorry it took me a while to get to this. Your @Retry(abortOn = NotFoundException.class) also effectively includes retryOn = Exception.class, which matches all exceptions and the cause chain doesn't get inspected at all. The behavior you observe is exactly as documented: https://smallrye.io/docs/smallrye-fault-tolerance/5.6.0/usage/extra.html#_inspecting_exception_cause_chains

That said, I agree that the documented behavior is pretty much useless when Retry.retryOn / Fallback.applyOn / CircuitBreaker.failOn is left to the default value (which is Exception.class or Throwable.class). In such case, the cause chain is always ignored.

I need to figure out a better definition of cause chain inspection. Filed smallrye/smallrye-fault-tolerance#768 for this. Thanks for the report!