OT: Best Practice to avoid null return values / get service errors into View

General
1

Hi everyone.

I have a question, little off topic but would like to hear your feedback/best practice and how you handle that. I am reading a little bit about clean code and found some methods where i return null in a method, which should be avoided in terms of clean code.

Imagine you have a Vaadin View. This View has a Rest API service injected and calls an external API. This API returns an object or thows errors when there is a problem in the API.

How do you handle messages/errors" to return them from the service back to the frontend layer to show kind of “user friendly” message

(following is dummycode)

class MyView {
//Vaadin View implementatoin

RestClient restClient;
//inject Restclient

   Button button = new Button ("Get Data");
   button.addClickListener {
   MyObject myObj = restClient.getData();
   if(myObj.getSuccess){
      //use object
    } else {
      if(myObj.getMessage.contains(XY))
      Notification.ErrorNotifitcation("Error xy").show();
   }
  }
}

Currently i wrap it in a class like:

public class CustomResonse {
private MyObject obj;
private boolean success;
private String message;
}

public class RestClient {
    public MyObject getData() {
        return restClient.post()
                .uri(baseUrl + "/externalEndpoint")
                .headers(httpHeaders -> {
                    httpHeaders.set("Content-Type", "application/json");
                    httpHeaders.set("Authorization", "Bearer " + validatedToken);
                })
                .body(body)
                .exchange((req, res) -> {
                    try {
                        if (res.getStatusCode().is2xxSuccessful()) {
                            MyObject mo = new MyObject();
                            mo.setSuccess(true);
                            return mo;
                        }
                    } catch (HttpClientErrorException e) {
                        e.printStackTrace();
                        MyObject mo = new MyObject();
                        mo.setSuccess(false);
                        mo.setMessage("error from exception");
                        return mo;
                    } catch (IOException re) {
                        MyObject mo = new MyObject();
                        mo.setSuccess(false);
                        mo.setMessage("error from exception");
                        return mo;
                    } catch (ResourceAccessException re) {
                        MyObject mo = new MyObject();
                        mo.setSuccess(false);
                        mo.setMessage("error from exception");
                        return mo;
                    }
                    MyObject mo = new MyObject();
                    mo.setSuccess(false);
                    mo.setMessage("error from exception");
                    return mo;
                });
    }
}

Would love to get your best practice :)

2

I personally would go with a monad pattern based on a custom wrapper class - like the Binder’s ValidationResult. You can get some inspiration how different level of it look here GitHub - tonivade/purefun: Functional Programming library for Java

Edit: similar to your code, but more generic

1 Like
3

There is no definite one right answer to this question, there are number of ways to achieve something that can be said to be clean code.

Avoiding null return values is not mandatory, but it is one option.

You can also embrace Java exception handling. My habit here is to let my service throw, and handle those exceptions in the presenter

And in case of exception I am showing something appropriate to the user. This is very clean.

But as pointed by @knoobie you can have a custom result type, which can convey different error cases.

If you like Kotlin style approach to nullability, then you could use some additional Java library like Jspecify.

In some cases null values can be seen useful as well.

What ever you do, select pattern you like, and use it consistently. I think the consistency is the actual clean thing here, not the specific pattern you use.

4

To me, it depends on how often this error path happens.

If it’s an exceptional problem that should not happen, then I will throw an exception.

If this happens often and you have more than one outcome, I would use a result class that contains the error.

The advantage of the result class is, as Knoobie mentioned, a clean, functional implementation.

5

Hm okay i think its kind of “usual” if the API is not available or if someone enter wrong login credentials for example.

I will go with the result class which holds success or not and a message if an error occurs. This error can be “translated” from frontend class into human readable error.

Would you agree?

6

I think this could be a good idea.
Btw calling APIs you should think about resilience patterns

7

with some API endpoints i consume, i am using @Retry where i know its a tempoary issue. But in this case the user most time use his portal password instead using a token, he received from the external provider. So i want to let him know “hey its 401 you have used wrong logindata”

8

I have another question which relates a little bit to this topic:

I have the method below and want to throw an own exception when getting 400 error and if the response contains special note.

I am very new to restClient (used resttemplate before) and confused, that it thows an ResourceAccessException when calling exchange() and i am not able to catch in in the exchange method.

I could surround the whole call into a try catch, but anyhow that feeld strange because i want to evaluate/analyze the exception with the body in the exchange method.

BTW some questions:

  • Can some one suggest a kind forum for spring or other general questions? I dont like Stackoverflow that much
  • may there is another libary/framework to handle rest calls instead of using restclient? What do you use?
 public boolean cancelBooking(String body, String baseUrl, String token) {
  boolean  success = restClient.post()
            .uri(baseUrl + "xxxx")
            .headers(
                    httpHeaders -> {
                        httpHeaders.set("Authorization", "Bearer " + token);
                    })
            .body(body)
            .exchange((req, res) -> {                                   // HERE THE EXCEPTION OCCURS
                try {
                    if (res.getStatusCode().is2xxSuccessful()) {
                        return true;
                    }
                    if(res.getStatusCode().is4xxClientError()){         //HERE I WANT TO HANDLE IT
                        if(res.getBody().toString().contains("xxx")){
                            throw new WrongStateException("xxx");
                        }
                    }
                } catch (HttpClientErrorException e) {
                    e.printStackTrace();
                } catch (IOException re) {
                    System.out.println(res.getBody());
                }
                return false;
            });
  return success;
}
9

Why do you use exchange? Couldn’t you just use the ResponseEntity?

The ResourceAccessException is thrown of an I/O exception happens. What’s the reason in your case?

11

I am new to RestClient and try a lot. I have tried .post & .retrieve as well but throwed also the exception. Whats the main difference to use .post() .get() in difference to use .exchange()?

Thought exchange is more handy to react to the response in the .exchange() Method

I will try if it change when i use ResponseEntity.

I dont understand, why it throws the ResourceaccessException (when i add the body) and get a 400 with a payload in the response “could not cancle booking, because of status” < this message is what i need and want to handle.

But when i dont send the body i get a 400 as well with the payload “body missing”, but no ResourceaccessException gets thrown.

In both cases i get a 400 with the exact same payload structure in the response, but one throws an Error, the other can be handled.

Same happens when not using .exchange():

public boolean cancelBooking(String myBody, String baseUrl, String token) {
    String success = restClient.post()
            .uri(baseUrl + "xxx")
            .body(myBody) //with this body it thows RAE, but the body is correct. The response says "you cant cacel that booking because its too late" thats what i expect and need to handle
            .retrieve()
            .onStatus(HttpStatusCode::is4xxClientError, (request, response) -> {
                String result = IOUtils.toString(response.getBody(), StandardCharsets.UTF_8);
                if (result.contains("Txxxd")) {
                    throw new WrongStateException("xxx");
                }
            })
            .body(String.class);
    System.out.println(success);
    return false;
}

In first case (with myBody) it crashes at retrieve() with the RAE and 400 and payload “not cancelable” But this is what i expect to get. When i send it without .body(myBody), i can handle the 400 Response in the onStatus().

when i send this request in Postman, i get in both cases also a 400 which i expect.

Could my Interceptor for Logging could be a reason? I will later try to send this without the logging interceptor.

12

Looks like my Logging Interceptor is the problem.

When i dont use it, everything works fine. Need to check the interceptor/BufferedClientHttpResponseWrapper

1 Like
13

@SimonMartinelli there is no content on your website/Youtube for logging or am i blind?;) Maybe an idea for a new blog post. The most tutorials out there are deprecated or only applicable with Spring RestTemplate not RestClient

14

What do you want to log?

Like this: Implementing Request/Response Logging with Spring Boot's Rest Client

15

Yep i am using this. Found it 5 minutes ago and it looks like its working, but cant log the headers. Where does logHeaders() come from?

16

I formated the output a little bit, so now i am able to log the headers see below

public class ClientLoggerRequestInterceptor implements ClientHttpRequestInterceptor {
    private static final Logger log = LoggerFactory.getLogger(ClientLoggerRequestInterceptor.class);

    @Override
    public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
        logRequest(request, body);
        var response = execution.execute(request, body);
        return logResponse(response);
    }

    private void logRequest(HttpRequest request, byte[] body) {
        log.debug("===========================REQUEST begin================================================");
        log.debug("URI         : {}", request.getURI());
        log.debug("Method      : {}", request.getMethod());
        log.debug("Headers     : {}", request.getHeaders());
        if (body != null && body.length > 0) {
            log.debug("Request body: {}", new String(body, StandardCharsets.UTF_8));
        }
        log.debug("==========================REQUEST end================================================");
    }

    private ClientHttpResponse logResponse(ClientHttpResponse response) throws IOException {
        log.debug("===========================RESPONSE begin================================================");
        log.debug("Status code  : {}", response.getStatusCode());
        log.debug("Status text  : {}", response.getStatusText());
        log.debug("Headers      : {}", response.getHeaders());
        byte[] responseBody = response.getBody().readAllBytes();
        if (responseBody.length > 0) {
            log.debug("Response body: {}", StreamUtils.copyToString(response.getBody(), Charset.defaultCharset()));
        }
        log.debug("===========================RESPONSE end================================================");

        // Return wrapped response to allow reading the body again
        return new BufferingClientHttpResponseWrapper(response, responseBody);
    }
}