quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.56k stars 2.63k forks source link

MP reactive client hides response error #17153

Closed GeauxEric closed 3 years ago

GeauxEric commented 3 years ago

Description

In my Quarkus microservice, I am using a microprofile rest client to fetch data from other external services:

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import io.smallrye.mutiny.Uni;

/**
 * A MicroProfile REST client of internal services APIs, such as xxx service, xxx service. To
 * configure the base URL, one needs to add a line to the application.properties file. 
 */
@Path("/api")
@ApplicationScoped
@RegisterRestClient
public interface InternalService {

  @GET
  @Path("data")
  Uni<byte[]> getData();
}

In case the API call fails, it always throws this error regardless of the actual endpoint:

{"message":"Handled
Internally","stackFrames":["org.jboss.resteasy.microprofile.client.ExceptionMapping$HandlerException:
Handled Internally","\tat
org.jboss.resteasy.microprofile.client.ExceptionMapping.filter(ExceptionMapping.java:72)","\tat
org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.filterResponse(ClientInvocation.java:715)","\tat
org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.invoke(ClientInvocation.java:489)","\tat

That makes it very difficult to debug the failure.

By reading the source code, I found the actual implemention:

/**
 * This implementation is a bit of a hack and dependent on Resteasy internals.
 * We throw a ResponseProcessingExceptoin that hides the Response object
 */
@SuppressWarnings({"rawtypes", "unchecked"})
public class ExceptionMapping implements ClientResponseFilter {
    public static class HandlerException extends ResponseProcessingException {
        protected ClientResponse handled;
        protected List<ResponseExceptionMapper> candidates;

I wonder if it is possible to configure the behavior of error handling of MP client.

geoand commented 3 years ago

Can you be a little specific about what you expect to see?

Thanks

GeauxEric commented 3 years ago

Can you be a little specific about what you expect to see?

Thanks

Instead of this default exception message:

{"message":"Handled
Internally","stackFrames":["org.jboss.resteasy.microprofile.client.ExceptionMapping$HandlerException:
Handled Internally","\tat
org.jboss.resteasy.microprofile.client.ExceptionMapping.filter(ExceptionMapping.java:72)","\tat
org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.filterResponse(ClientInvocation.java:715)","\tat
org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.invoke(ClientInvocation.java:489)","\tat

I wish to see the actual response in case of failure.

geoand commented 3 years ago

@michalszynkiewicz I don't know if ^ is doable easily.

@EricTing have you perhaps tried the new quarkus-rest-client-reactive?

michalszynkiewicz commented 3 years ago

You should be able to create your own ResponseExceptionMapper that would have access to the repsonse object, see https://download.eclipse.org/microprofile/microprofile-rest-client-2.0/microprofile-rest-client-spec-2.0.html#_responseexceptionmapper

GeauxEric commented 3 years ago

@michalszynkiewicz I don't know if ^ is doable easily.

@EricTing have you perhaps tried the new quarkus-rest-client-reactive?

Yes. I recently updated to 1.13 and quarkus-rest-client-reactive effectuated. Previously the client was blocking. See https://github.com/quarkusio/quarkus/issues/3084

GeauxEric commented 3 years ago

You should be able to create your own ResponseExceptionMapper that would have access to the repsonse object, see https://download.eclipse.org/microprofile/microprofile-rest-client-2.0/microprofile-rest-client-spec-2.0.html#_responseexceptionmapper

Thanks, let me try it out. May come back later to this issue.

GeauxEric commented 3 years ago

Thanks for the pointer. Adding my own ResponseExceptionMapper works.

Sharing my code snippet here:

@Priority(Priorities.USER)
public class RestClientExceptionMapper
    implements ResponseExceptionMapper<InternalServiceException> {

  @Override
  public InternalServiceException toThrowable(Response response) {
    try {
      response.bufferEntity();
    } catch (Exception ignored) {
    }
    String msg = getBody(response);
    return new InternalServiceException(msg);
  }

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

  private String getBody(Response response) {
    ByteArrayInputStream is = (ByteArrayInputStream) response.getEntity();
    byte[] bytes = new byte[is.available()];
    is.read(bytes, 0, is.available());
    String body = new String(bytes);
    return body;
  }
}
// to use it
@Path("/api")
@ApplicationScoped
@RegisterRestClient
@RegisterProvider(value = RestClientExceptionMapper.class)
public interface InternalService {

Pls feel free to close this ticket.

quarkus-bot[bot] commented 3 years ago

/cc @michalszynkiewicz

ghost commented 3 years ago

This doesn't seems to be working on quarkus 2.1.1.Final.

I just used the given example and still I get :

org.jboss.resteasy.microprofile.client.ExceptionMapping$HandlerException: Handled Internally

My ResponseExceptionMapper is registered on my RestClient Interface, its being called (method handles) but the toException is never called. The org.jboss.resteasy.microprofile.client.HandlerException is always thrown. Any ideas? Using quarkus-rest-client.

Thanks.

michalszynkiewicz commented 3 years ago

@casmeiron are you sure handles returns true in your exception mapper? Also, does your mapper have a higher priority than the default exception mapper?

ghost commented 3 years ago

Yes, handles returns true, higher priority etc... I've changed to reactive version, now it works like it should.

michalszynkiewicz commented 3 years ago

@casmeiron what do you mean by changing to reactive version?

ghost commented 3 years ago

@michael-schnell Changed to -Dextensions="resteasy-reactive-jackson,rest-client-reactive-jackson" fixed the issue for me.

jciombalo commented 4 months ago

Hi,

I'm using quarkus v. 3.10.1 with quarkus-rest-jackson and quarkus-rest-client-jackson extensions.

I have a service A that makes client rest requests to another service B. The B service is returning an HTTP 403 Forbidden as expected based on the authentication scenario.

Based on the guides I've created in the A service a ClientExceptionMapper to re-throw the 4XX status errors back to the client, otherwise it was returning a 500 Internal server error.

Now, if try to call the B service with procedural programming, it is returnning status codes from the B service to the client as expected. However, when i try the same approach using reactive request, the exception mapper toThrowable is being called, but the response from A to client is still an http status code 500 error.

Is there any trick to make the exception mapper work with the reactive client? Am I missing something from the docs?

This is my mapper implementation:

@Provider
public class ClientExceptionMapper implements ResponseExceptionMapper<RuntimeException> {

    @Inject
    Logger logger;

    @Override
    public boolean handles(int status, MultivaluedMap<String, Object> headers) {
        return status >= 400 && status < 500;
    }

    @Override
    public RuntimeException toThrowable(Response response) {
        return new ClientErrorException(response);
    }
}

This is the Façade for the B service:

@RegisterRestClient(configKey = "my-second-level-service")
@Path("")
@RegisterClientHeaders
public interface MySecondLevelServiceFacade {

    @GET
    @Path("/echo")
    String echo(String value);

    @GET
    @Path("/echo")
    Uni<String> echoAsync(String value);

}

This is the service currently being tested:

public class MyFirstLevelServiceResource {

    @RestClient MySecondLevelServiceFacade secondLevelService;

    @GET
    @Path("/echo")
    String echo(@RestQuery String value) {
        return secondLevelService.echo(value);
    } 

    @GET
    @Path("/echo/async")
    Uni<String> echoAsync(@RestQuery String value) {
        return secondLevelService.echoAsync(value);
    }

EDIT: This is the response body that I get from the MyFirstLevelServiceResource.echoAsync along with the 500 status code:

{
  "details": "Error id f0b548d2-c67a-4430-87c2-8a13629fc0b8-1, org.jboss.resteasy.reactive.ClientWebApplicationException: Received: 'HTTP 401 Unauthorized' when invoking: Rest Client method: 'org.acme.MySecondLevelServiceFacade#echoAsync'",
  "stack": "org.jboss.resteasy.reactive.ClientWebApplicationException: Received: 'HTTP 401 Unauthorized' when invoking: Rest Client method: 'org.acme.MySecondLevelServiceFacade#echoAsync'\r\n\tat org.jboss.resteasy.reactive.client.impl.RestClientRequestContext.unwrapException(RestClientRequestContext.java:195)\r\n\tat org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.handleException(AbstractResteasyReactiveContext.java:329)\r\n\tat org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:175)\r\n\tat org.jboss.resteasy.reactive.client.impl.RestClientRequestContext$1.lambda$execute$0(RestClientRequestContext.java:314)\r\n\tat io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:279)\r\n\tat io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:261)\r\n\tat io.vertx.core.impl.ContextInternal.lambda$runOnContext$0(ContextInternal.java:59)\r\n\tat io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)\r\n\tat io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)\r\n\tat io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)\r\n\tat io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:566)\r\n\tat io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)\r\n\tat io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)\r\n\tat io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)\r\n\tat java.base/java.lang.Thread.run(Thread.java:1583)\r\nCaused by: jakarta.ws.rs.ClientErrorException: HTTP 401 Unauthorized\r\n\tat org.acme.ClientExceptionMapper.toThrowable(ClientExceptionMapper.java:25)\r\n\tat org.acme.ClientExceptionMapper.toThrowable(ClientExceptionMapper.java:12)\r\n\tat io.quarkus.rest.client.reactive.runtime.MicroProfileRestClientResponseFilter.filter(MicroProfileRestClientResponseFilter.java:52)\r\n\tat org.jboss.resteasy.reactive.client.handlers.ClientResponseFilterRestHandler.handle(ClientResponseFilterRestHandler.java:21)\r\n\tat org.jboss.resteasy.reactive.client.handlers.ClientResponseFilterRestHandler.handle(ClientResponseFilterRestHandler.java:10)\r\n\tat org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.invokeHandler(AbstractResteasyReactiveContext.java:231)\r\n\tat org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)\r\n\t... 12 more"
}
geoand commented 4 months ago

Do you have a sample application I can run myself showing what you describe?

jciombalo commented 3 months ago

Nevermind, I've just realized that my scenario is the one discussed here: #37029