Reputation: 9325
Let say I develop some rest api. I have web layer (controllers), and service layer (model). Is it good practice to throw exception with HttpStatus code in service layer?
Someone will say, that model should not know anything about web layer, it should not depend on web layer.
But, let's consider this situations:
Throwing exceptions with HttpStatus in controller
--- Service ---
class userService {
public void updateUser(int userId, String username, String email) {
if ( //userid not found) {
throw new UserNotFoundException("User not found");
}
if ( // bad email format) {
throw new BadArgumentException("Bad email format");
}
if ( // user is not active) {
throw new AccessDeniedException("User is not active");
}
... here save user ...
}
}
--- Controller ---
class UserController {
@RequestMapping ....
public void updateUser(int id, String username, String email) {
try{
userService.updateUser(id, username, email);
}
catch (UserNotFoundException e) {
throw new HttpException(e.getMessage(), HttpStatus.NOT_FOUND);
}
catch (BadArgumentExceptione e) {
throw new HttpException(e.getMessage(), HttpStatus.BAD_REQUEST);
}
catch (AccessDeniedException e) {
throw new HttpException(e.getMessage(), HttpStatus.FORBIDEN);
}
}
}
You see? How many extra code should I write to return proper response to Api client? In addition, I can forget to catch some exception which could be reported properly, and it will be returned as default Internal Server Exception. In controller I always should look in to the service layer and check what exceptions can throw service to properly handle them. (Please do not suggest checked exception in java).
And now let's see another solution:
Throwing exceptions with HttpStatus in Service Layer (model)
--- Service ---
class userService {
public void updateUser(int userId, String username, String email) {
if ( //userid not found) {
throw new UserNotFoundException("User not found", HttpStatus.NOT_FOUND);
}
if ( // bad email format) {
throw new BadArgumentException("Bad email format", HttpStatus.BAD_REQUEST);
}
if ( // user is not active) {
throw new AccessDeniedException("User is not active", HTTP_STATUS.FORBIDEN);
}
... here save user ...
}
}
-- Controller --
class UserController {
@RequestMapping ....
public void updateUser(int id, String username, String email) {
userService.updateUser(id, username, email);
}
}
And that's it. Less code. Now I do not have to handle each possible exception which can throw Service layer, I don't have to check service exceptions manually each time when I write controller and I will not forget to handle some exception (because their properly formed in service layer) and that's less error prone.
And again, is this bad practice to handle Http related data in service layer?
If it's bad, how you will handle issus which I described.
Thank you.
P.S.: In both solutions, some general ErrorHandler catches exceptions and forms response with appropriate status codes.
Upvotes: 4
Views: 2410
Reputation: 8324
You can use @ExceptionHandler in a method inside your Controller to manage the exception in the same controller
@ExceptionHandler({MyException.class,OtherException.class})
public String myMethodExceptionHandler() {...}
or you can create a class with @ControllerAdvice to manage the errors from all yours controllers
@ControllerAdvice
class GlobalControllerExceptionHandler {
@ResponseStatus(HttpStatus.CONFLICT) // 409
@ExceptionHandler(MyException.class)
public void handleConflict() {
// Nothing to do
}
}
Here there is a tutorial. https://spring.io/blog/2013/11/01/exception-handling-in-spring-mvc
Upvotes: 3