Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REST Client (classic) - exception mapper not honored with async return types (CompletionStage and Uni) #21175

Closed
michalszynkiewicz opened this issue Nov 3, 2021 · 9 comments
Labels
area/rest-client area/resteasy kind/bug Something isn't working triage/wontfix This will not be worked on

Comments

@michalszynkiewicz
Copy link
Member

Describe the bug

from https://stackoverflow.com/questions/69825357/error-handling-on-quarkus-mutiny-rest-client:

On my quarkus rest project i have a restclient that uses mutiny:

@Path("/")
@RegisterRestClient(configKey = "my-api")
@RegisterClientHeaders
@RegisterProvider(MyExceptionMapper.class)
public interface MyClient {

  @POST
  @Path("path")
  Uni<MyBean> get(String body);
}

I wanna handle propery non 2XX httpError so i have made my ExceptionMaper

public class MyExceptionMapper implements ResponseExceptionMapper<MyException> {
  @Override
  public MyException toThrowable(Response response) {
    //TODO
    return new MyException();
  }
}

a bad call on the client shows that MyExceptionMapper handle the response but the exception raises and does not became a failure on my Uni Client response object

Uni<MyBean> bean = myClient.get("") // i do not have a failure in case of 4XX http 
   .onFailure().invoke(fail -> System.out.println("how can i get here?"));

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.4.0.Final

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

maven

Additional information

No response

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 3, 2021

/cc @cescoffier, @jponge

@nikosk686
Copy link

I am suffering from the same issue and I can verify that the workaround is switching from quarkus-rest-client-mutiny to quarkus-rest-client-reactive as suggested in the answer of the linked SO question.

@cescoffier
Copy link
Member

I just tried with the Reactive REST Client and the mapper is called.
I will try with the non-reactive one.

@cescoffier cescoffier changed the title REST Client Mutiny - exception mapper not honored with Uni return type REST Client (classic) - exception mapper not honored with async return types (CompletionStage and Uni) Jan 24, 2022
@cescoffier
Copy link
Member

It's not an issue related to Mutiny. Same behavior when returning a CompletionStage.
The mapper is called, but the exception is then swallowed.

@cescoffier
Copy link
Member

I've found the issue.

In org.jboss.resteasy.microprofile.client.ExceptionMapping.HandlerException the mapException method checks the exception declared in the method signature. While in imperative it works, as the method signature must declare the exception as part of the signature, it's not the case in the async world (exception, aka failures, are always expected).

To get the example working, you need to declare the exception explicitly, even if it does not make sense.

  @POST
  @Path("path")
  Uni<MyBean> uni(String body) throws MyException;

  @POST
  @Path("path")
  CompletionStage<MyBean> cs(String body) throws MyException;

The caller would need to catch or declare the exception despite it cannot happen.

The fix (that would be in the main RESTEasy codebase) should verify if the mehtod is async and in this case, skip the following check:

for (Class exc : method.getExceptionTypes()) {
                    if (exception != null && exc.isAssignableFrom(exception.getClass())) {

This is only for RESTEasy classic. Everything works as expected in Reasteasy Reactive.

@geoand
Copy link
Contributor

geoand commented May 17, 2022

I am going to close this as won't fix since all our efforts around the rest-client are focused on the reactive rest client, which as mentioned above works perfectly in this scenario

@geoand geoand closed this as completed May 17, 2022
@geoand geoand added the triage/wontfix This will not be worked on label May 17, 2022
@gsmet
Copy link
Member

gsmet commented May 17, 2022

Pinging @jamezp anyway as it might be of interest for RESTEasy. @jamezp see the detailed comment from @cescoffier above ^.

@jamezp
Copy link
Contributor

jamezp commented May 17, 2022

Thanks @gsmet

@nikosk686
Copy link

Looks like that the issue has been fixed anyway, I ve upgraded to the latest quarkus version (2.10.2 atm) and my
ResponseExceptionMapper seems to be working as expected with quarkus-rest-client-mutiny

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest-client area/resteasy kind/bug Something isn't working triage/wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

6 participants