@Rest and RestClientException handler #623

Activity

1. DayS commented on Jun 9, 2013

   

   DayS

   on Jun 9, 2013

   Contributor

   More actions

   I also use a wrapper with AOP to handle exception on every rest calls.
   This issue is somehow related to #508 but you idea is interesting. We could get rid of our wrappers with this :)

   The annotated code could looks like this :

   @Rest(converters = MappingJacksonHttpMessageConverter.class)
   public interface RestClient {
       @Get("/")
       String test();
   
       void errorHandler(RestErrorHandler hanlder);
   }

   The RestErrorHandler interface :

   public interface RestErrorHandler {
       void onRestClientExceptionThrowed(RestClientException e);
   }
2. DayS commented on Jun 9, 2013

   

   DayS

   on Jun 9, 2013

   Contributor

   More actions

   It could also be handled by #339
3. rockytriton commented on Jun 10, 2013

   

   rockytriton

   on Jun 10, 2013

   Contributor

   More actions

   I like this idea, what do you think about the last comment I made on #339? We could add this errorHandler() method to that interface as well.
4. rockytriton commented on Jul 13, 2013

   

   rockytriton

   on Jul 13, 2013

   Contributor

   More actions

   So, when an exception is thrown, what should be returned from the method call?

   I assume the method call goes something like this:

   public String getCustomer(String customerId) {
   ...
           try {
               return restTemplate.exchange(rootUrl.concat("/rest/{customerId}"), HttpMethod.GET, null, String.class, urlVariables).getBody();
           }
           catch(RestClientException e) {
               if (restErrorHandler != null) {
                   restErrorHandler.onRestClientExceptionThrown(e);
                   return null; //????
               }  else {
                   throw e;
               }
           }
   }

   So I guess we would have one of two options, return null (as shown in the example) or rethrow the exception after calling the handler.
5. DayS commented on Jul 13, 2013

   

   DayS

   on Jul 13, 2013

   Contributor

   More actions

   Maybe the result could be defined by the error handler.
   The code could looks like this, so :

   public interface RestErrorHandler {
       Object onRestClientExceptionThrowed(RestClientException e);
   }

   And the generated code :

   public String getCustomer(String customerId) {
       // ...
       try {
           return restTemplate.exchange(rootUrl.concat("/rest/{customerId}"), HttpMethod.GET, null, String.class, urlVariables).getBody();
       } catch(RestClientException e) {
           if (restErrorHandler != null) {
               return restErrorHandler.onRestClientExceptionThrown(e);
           }  else {
               throw e;
           }
       }
   }

   I'm not sure if it's the best solution. Any ideas ?
6. rockytriton commented on Jul 13, 2013

   

   rockytriton

   on Jul 13, 2013

   Contributor

   More actions

   I don't think this will be possible, the return value of the rest method is user defined. In this case it's string, but it could be something like CustomerResponse or whatever other object the service returns. I tried playing around with returning null and it seems to work fine. It just returns null back as the object, so the developer can check that return value for null.
7. tbruyelle commented on Jul 13, 2013

   

   tbruyelle

   on Jul 13, 2013

   ContributorAuthor

   More actions

   I like the idea that the handler returns the same object than the method, because the method caller has a better understanding of what happens. Moreover it allows to use same error handling code between network and server error.

   But to achieve that, we have to impose the user to use an AA interface like RestClientResponse<?> for his RestClient methods. The object would encapsulate the server response and would have methods like hasError() or isSuccess().
   But probably I'm going to far and it will complexify too much the usage of @RestClient...
8. rockytriton commented on Jul 13, 2013

   

   rockytriton

   on Jul 13, 2013

   Contributor

   More actions

   Also I don't think it would be backward compatible, I see where you are going with it though. I think returning null would probably be the best bet now so that it doesn't make the rest client more complex to use and stays backward compatible with previous versions.
9. tbruyelle commented on Jul 14, 2013

   

   tbruyelle

   on Jul 14, 2013

   ContributorAuthor

   More actions

   Yes you're right.
10. DayS commented on Jan 20, 2014

    

    DayS

    on Jan 20, 2014

    Contributor

    More actions

    This feature has been implemented in 3.0
11. DayS

    closed this as completedon Jan 20, 2014