Approach to handle exception in Spring Boot. Throw exception catch and throw the same exception, is it a bad practice?

Question

I'm working with spring boot and I'm dealing with a good way to handle exceptions:

I have this case where interact with a repository to get a product. But what about if there is a connection issue with the DB, I'm not going to catch that exception

Product product = productRepository.findById(productId)
                .orElseThrow(() -> new NotFoundException("Could not find a product"));
try {
// Product logic
} catch(Exception e) {
            throw new ProductException(String.format("Error getting product productId=%s. Exception message: %s",
                    productId, e.getMessage()), e);
        }

I have a controller advice to catch the exception and return a nice response:

@ControllerAdvice
public class ExceptionHandler extends ResponseEntityExceptionHandler {
    @ExceptionHandler({NotFoundException.class})
    public ResponseEntity<Error> handleNotFoundException(HttpServletRequest request, Exception exception) {
    .....
}

I think I could do something like this:

    try {
        Product product = productRepository.findById(productId)
                    .orElseThrow(() -> new NotFoundException("Could not find a product"));
    } catch(NotFoundException e) {
        throw new NotFoundException(e.getMessage())
    } catch(Exception e) {
                throw new ProductException(String.format("Error getting product productId=%s. Exception message: %s",
                        productId, e.getMessage()), e);

That worked but it looks weird since I'm throwing NotFoundException twice. Any idea?

score 4 · Accepted Answer · answered Jul 20 '20 at 23:02

Generally speaking, there is nothing weird catching an Exception just to throw it again right after. You may for example catch it to log it somewhere and then throw it to the next handler for real handling (like in your case).

You can simply write:

catch (NotFoundException e) {
    // Log if needed
    throw e; // <-- no need to build a new one
}

However, for the part which comes after, catching generic Exception is usually a bad practice. There are too many sources this exception may come from, and you can't just assume that it is because of the DB not responding.

So the best would be that your function catches what it knows how to handle, and throws what it doesn't know how to handle. For example:

public void yourFunction() throws NotFoundException, ProductException {
    //you don't know how to handle NotFoundException and ProductException here, hence you throw them up
    try {
        // your logic
    } catch (SpecificExceptionYouWantToCatch e) {
        //handle specific exception you know how to handle
    } catch (AnotherSpecificException e) {
        //handle other specific exception you know how to handle
    }
    //Let all the other Runtime exceptions flow up to the caller. 
}

If there is an exception, something went wrong. You can't just wrap it and pretend it was a ProductException, so:

If you recognize the Exception and know how to handle it, then do it (as you're doing for NotFoundException)
However, if you didn't expect that Exception, then better let it flow. At some point it will be caught and handled, maybe, or maybe will just crash the program. But you will want to know it, while if you just wrap it into a ProductException you will probably hide to the system a bigger issue.

Ok got it.... what about catch(Exception e) log all related to the exception and then throw an exception to build a more friendly response. This way I'll keep the error message in the api and won't all the horrible stack to the client. What do you think? — Julio, Jul 20 '20 at 23:37
@Julio yes, that's pretty common practice if your function is an API endpoint. But it will be an Internal Server Error, not a Product Exception. Also, be careful: you want to log the stack in the log trace (because you or the technical operator will need to see it), however you don't want to expose it to the API user (because you don't want to leak your logic to them). — Matteo NNZ, Jul 20 '20 at 23:40
yes great, I think that part could be solved adding some alerts to monitor the service. Thanks so much — Julio, Jul 20 '20 at 23:41
P.s. stacks are beautiful to the eyes of a developer, don't call them horrible :) — Matteo NNZ, Jul 20 '20 at 23:42

score 0 · Answer 2 · answered Jul 20 '20 at 23:02

You can still catch it in ControllerAdvice, even you don't throw it so there is no need to try caatch that exception instead add:

@ExceptionHandler({PSQLException.class})

This is for Postgres you will have different exception for other typoes of databases then you can handle the message as well and see what is the cause of it in ControllerAdvice

Approach to handle exception in Spring Boot. Throw exception catch and throw the same exception, is it a bad practice?

2 Answers2