Reactive Rest Client should allow extracting response status from WebClientApplicationException

[andreas-eberle]

Describe the bug

When the rest client makes a call to an external service, that call can fail with a non-OK status code (e.g. 401 when a token timed out). In such a case, the reactive rest client creates a WebClientApplicationException (see

quarkus/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSetResponseEntityRestHandler.java

Line 24 in e21b6c2

throw new WebClientApplicationException("Server response status was: " + context.getResponseStatus());

). However, it only provides a message with the status code to the exception.

Thus, the only way to find out the status code of the response of the call is to parse the message of the WebClientApplicationException. In my case I need the status because I implement a service similar to a "proxy" and therefore need to return the same status code from my service to my clients.

Is there a reason not to use the standard javax.ws.rs.WebApplicationException? If I recall correctly, that was used by the standard rest client.

Furthermore, there is a difference in the type of Exception you get depending on the return type of your rest client method. If you return a Uni<MyDto> and the call receives a non-OK status, you get a WebClientApplicationException. However, if you return a Uni<Response> in the rest client method, you get a ProcessingException that contains a standard javax.ws.rs.WebApplicationException. See the reproducer for more details.

Expected behavior

1. It must be easily possible to get the causing status code from the thrown exception.
2. You should get the same error no matter if the rest client method returns a Uni<Dto> or Uni<Response>.

Actual behavior

1. The WebClientApplicationException only contains the status code in the message from which we would need to parse it.
2. The thrown exception differs depending on the return type of your rest client method.

To Reproduce

1. Download and unzip reproducer project
   2021-05-20-reactive-restclient-exceptions.zip.
2. Start the project with ./gradlew quarkusDev
3. Open your browser and open http://localhost:8080/test-dto . This will make a call with the rest client which receives a 401 status. The rest client method has a return Type of Uni<Dto>. You will receive the following error object:

{"exceptionClass":"WebClientApplicationException","message":"Message: 'Server response status was: 401' ;;; Response Object: null"}

4. Open your browser and open http://localhost:8080/test-response . This makes a call with the rest client and also receives a status 401. However, this time the rest client method returns Uni<Response>. You will now receive the following error object:

{"exceptionClass":"ProcessingException","message":"javax.ws.rs.WebApplicationException: Unknown error, status code 401"}

Additional context

• Tested with Quarkus 1.13.4.Final.
• In main, the WebClientApplicationException also only takes the message argument. So issue 1 persists. Issue 2 I haven't tested yet.

[quarkus-bot]

/cc @michalszynkiewicz

[andreas-eberle]

@geoand: you appeared in the git blame ;) Thanks for your help!

[geoand]

I'll have a look

[geoand]

First of all I opened #17402 to make the exception more usable.

However, since the exception handling behavior of the client has changed significantly in 2.x (to be better aligned with the MicroProfile REST Client spec), you will basically need to throw this exception (or any custom exception you like) in an org.eclipse.microprofile.rest.client.ext.ResponseExceptionMapper.

It would look something like this:

class WebClientApplicationExceptionMapper : ResponseExceptionMapper<WebClientApplicationException> {

    override fun toThrowable(response: Response?) = WebClientApplicationException(response!!.statusInfo.statusCode, response!!.statusInfo.reasonPhrase)
}

and you would add @RegisterProvider(WebClientApplicationExceptionMapper::class) to your TestApiRestClient.

Then your server exception mapper (ApplicationExceptionMapper ) would need to handle ProcessingException (@michalszynkiewicz is this needed to satisfy the MP REST Client spec?) and pull the real exception out of it, something like:

val (status, message: String) = when {
            exception is WebApplicationException && exception.response != null && exception.response.status != 500 -> {
                exception.response.status to exception.localizedMessage
            }
            exception is ProcessingException -> {
                val cause = exception.cause
                if (cause is WebClientApplicationException) {
                    cause.response.status to cause.localizedMessage
                } else {
                    Response.Status.INTERNAL_SERVER_ERROR.statusCode to exception.localizedMessage
                }
            }
            else -> {
                log.log(Level.SEVERE, "Internal server error: ", exception)
                Response.Status.INTERNAL_SERVER_ERROR.statusCode to exception.localizedMessage
            }
        }

[andreas-eberle]

Thanks @geoand for the super fast response and PR!

Do I understand you correctly that in Quarkus 2.0, we will always have to implement our own ResponseExceptionMapper? Won't there be a default implementation?

Regarding the ProcessingException wrapping everything: Woudn't this cause problems with fault tolerance? E.g. with the @Retry, we can specify exceptions that trigger the retry. However, if there is only a single ProcessingException, you cannot differentiate between all the different exceptions that you might want to throw in the ResponseExceptionMapper.

[geoand]

Thanks @geoand for the super fast response and PR!

Do I understand you correctly that in Quarkus 2.0, we will always have to implement our own ResponseExceptionMapper? Won't there be a default implementation?

You don't to implement it. You only implement it if you want to convert a response code received from the client to a specific exception.

Regarding the ProcessingException wrapping everything: Woudn't this cause problems with fault tolerance? E.g. with the @Retry, we can specify exceptions that trigger the retry. However, if there is only a single ProcessingException, you cannot differentiate between all the different exceptions that you might want to throw in the ResponseExceptionMapper.

I can't comment on this as I don't know why ProcessingException is being used. That's for @michalszynkiewicz to clarify

[michalszynkiewicz]

If the interface method is declared to throw some exception, an exception mapper can return such an exception and it shouldn't be wrapped in anything IIRC.