Correct way to handle exceptions in Spring Boot

Question

I was reading the Spring docs and found that creating a subclass from ResponseEntityExceptionHandler was a good way on handling exceptions. However, I tried to handle exceptions in a different way, since I need to diff BusinessExceptions from TechnicalExceptions.

Created a bean called BusinessFault which encapsulates the exception details:

BusinessFault.java

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.annotation.JsonProperty;

@JsonInclude(value = Include.NON_NULL)
public class BusinessFault {

    @JsonProperty(value = "category")
    private final String CATEGORY = "Business Failure";
    protected String type;
    protected String code;
    protected String reason;
    protected String description;
    protected String instruction;

    public BusinessFault(String type, String code, String reason) {
        this.type = type;
        this.code = code;
        this.reason = reason;
    }

    public BusinessFault(String type, String code, String reason, String description, String instruction) {
        this.type = type;
        this.code = code;
        this.reason = reason;
        this.description = description;
        this.instruction = instruction;
    }

    public String getType() {
        return type;
    }

    public void setType(String type) {
        this.type = type;
    }

    public String getCode() {
        return code;
    }

    public void setCode(String code) {
        this.code = code;
    }

    public String getReason() {
        return reason;
    }

    public void setReason(String reason) {
        this.reason = reason;
    }

    public String getDescription() {
        return description;
    }

    public void setDescription(String description) {
        this.description = description;
    }

    public String getInstruction() {
        return instruction;
    }

    public void setInstruction(String instruction) {
        this.instruction = instruction;
    }

    public String getCATEGORY() {
        return CATEGORY;
    }
}

Created a BusinessException class, which do the job by creating a BusinessFault beans through the details passed by its constructor:

BusinessException.java

import com.rest.restwebservices.exception.fault.BusinessFault;

public abstract class BusinessException extends RuntimeException {

    private BusinessFault businessFault;

    public BusinessException(String type, String code, String reason) {
        this.businessFault = new BusinessFault(type, code, reason);
    }

    public BusinessException(String type, String code, String reason, String description, String instruction) {
        this.businessFault = new BusinessFault(type, code, reason, description, instruction);
    }

    public BusinessException(BusinessFault businessFault) {
        this.businessFault = businessFault;
    }

    public BusinessFault getBusinessFault() {
        return businessFault;
    }

    public void setBusinessFault(BusinessFault businessFault) {
        this.businessFault = businessFault;
    }
}

Created a specific UserNotFoundException class, which extends from BusinessException class:

UserNotFoundException.java

import com.rest.restwebservices.exception.fault.BusinessFault;
import com.rest.restwebservices.exception.map.ExceptionMap;

public class UserNotFoundException extends BusinessException {

    public UserNotFoundException(BusinessFault businessFault) {
        super(businessFault);
    }

    public UserNotFoundException(String reason) {
        super(ExceptionMap.USERNOTFOUND.getType(), ExceptionMap.USERNOTFOUND.getCode(), reason);
    }

    public UserNotFoundException(String reason, String description, String instruction) {
        super(ExceptionMap.USERNOTFOUND.getType(), ExceptionMap.USERNOTFOUND.getCode(), reason, description,
                instruction);
    }
}

Created a BusinessExceptionHandler, but instead of being a subclass of ResponseEntityExceptionHandler, it's only has a @ControllerAdvice annotation and a method that handles all thrown BusinessExceptions:

BusinessExceptionHandler.java

import javax.servlet.http.HttpServletRequest;

import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody;

import com.rest.restwebservices.controller.UserController;
import com.rest.restwebservices.exception.BusinessException;
import com.rest.restwebservices.exception.fault.BusinessFault;

@ControllerAdvice(basePackageClasses = UserController.class)
public class BusinessExceptionHandler {

    @ExceptionHandler(BusinessException.class)
    @ResponseBody
    public ResponseEntity<BusinessFault> genericHandler(HttpServletRequest request, BusinessException ex) {
        return new ResponseEntity<BusinessFault>(ex.getBusinessFault(), HttpStatus.OK);
    }
}

The service layer can throw a UserNotFoundException:

@Service
public class UserService {

    @Autowired
    private UserRepository userRepository;

    public User findById(Long id) {
        User user = userRepository.findOne(id);
        if (user == null)
            throw new UserNotFoundException("The ID " + id + " doesn't behave to any user!");

        return user;
    }
}

It works fine. But I was wondering if this is a bad practice on handling exceptions?

Answer 1

I've got a little problem with your exception handling. Principally it is absolutely ok to catch runtime exceptions, handle them and send them forth to the client, which is probably someone using your REST service and getting the error response as a JSON object. If you manage to tell him what he did wrong and what he can do about it, great! Of course, it will add some complexity to it, but it is probably easy and comfortable to work with that API.

But think about the backend developers, too, that work with your code. Especially the public User findById(Long id) method in your UserService is obscure. The reason for this is that you made your BusinessException, in particular, the UserNotFoundException unchecked.

If I joined your (backend) team, and I was to write some business logic using that service, I'd be quite sure what I had to expect from that method: I pass a user ID and get back a User object if it was found or null if not. That's why I would write code like that

User user = userService.findById("42A");
if (user == null) {
  // create a User or return an error or null or whatever
} else {
  // proceed
}

However, I would never know, that the first condition will never be true since you never return null. How should I know that I had to catch an Exception?

Is the compiler telling me to catch it? No, as it is not checked.

Would I look into your source code? Hell, no! Your case is extremely simple. That UserNotFoundException may be raised in another method in another class among hundred lines of code. Sometimes I couldn't look inside it, anyway, as that UserService is just a compiled class in a dependency.

Do I read the JavaDoc? Hahaha. Let's say, 50% of the time I wouldn't, and the other 50% you've forgotten to document it, anyway.

So, the developer has to wait until his code is used (either by a client or in Unit tests) to see that it doesn't work as he intended, forcing him to redesign what he has coded so far. And if your whole API is designed that way, that unchecked exceptions pop out of nowhere, it can be very very annoying, it costs time and money and is so easy to avoid, actually.