0

I have already gone through official Spring Boot tutorial, a couple of Stack Overflow posts including this, tried @Ordered(Order=HIGHEST_PRECEDENCE) but any exception other than the ConstraintViolationException.class is being handled by DefaultHandlerExceptionResolver

VERSIONS

  • Spring Boot - 2.5.3
  • Spring - 5.3.9

PROBLEM

I want any exception in MyRestController be handled by handlers in ControllerAdvice defined by me so that I can manipulate the response object before sending to the client.

@Validated
@RestController
@RequestMapping(value = "/myctx", produces = MediaType.APPLICATION_JSON_VALUE)
public class MyRestController {

    private static final Logger logger = LoggerFactory.getLogger(MyRestController);

    @RequestMapping(
            path = "/{element}",
            method = RequestMethod.POST,
            consumes = MediaType.APPLICATION_JSON_VALUE,
            produces = MediaType.APPLICATION_JSON_VALUE)
    public ResponseEntity<ApiResponse> findAndSave(
            @Allowed(values = { "a1", "a2", "a3" }) @PathVariable String element,
            @ValidateMetadata @RequestBody Metadata metadata) {
                ...
                ...
    }
}

The annotations @Allowed and @ValidateMetadata defined by me are working as expected

@ControllerAdvice
//@Order(Ordered.HIGHEST_PRECEDENCE)
public class ErrorControllerAdvice extends ResponseEntityExceptionHandler {

    private static final Logger logger = LoggerFactory.getLogger(ErrorControllerAdvice.class);

    @ExceptionHandler({ ConstraintViolationException.class })
    public ResponseEntity<Object> handleConstraintViolationException(ConstraintViolationException ex, WebRequest request) {
        ApiResponse apiResponse = ApiResponse.builder()
                .timestamp(new Date())
                .status(HttpStatus.BAD_REQUEST)
                .message(ex.getLocalizedMessage())
                .path(((ServletWebRequest) request).getRequest().getRequestURI())
                //.metadata(metadata)
                .build();

        return handleExceptionInternal(ex, apiResponse, new HttpHeaders(), apiResponse.getStatus(), request);
    }

    @ExceptionHandler({ Exception.class })
    public ResponseEntity<Object> handleAll(Exception ex, WebRequest request) throws Exception {
        ResponseEntity<Object> tmpResponse = super.handleException(ex, request);
        ResponseEntity<Object> response = new ResponseEntity<>("Response from catch-all exception handler", tmpResponse.getHeaders(), tmpResponse.getStatusCode());

        return response;
    }
}

At this point when I test my endpoint with a POST request and inject a ContraintViolation, my custom error response is sent to the client. However, for any other Exception, handleAll() method in ErrorControllerAdvice is not even invoked. For example, from the test code, I try to send a GET instead of a POST or change the URI to contain myct instead of myctx

Why is it not working? Why do I get default error messages in case of the situations described above?

Update

I even tried throwing a NullPointerException from within the controller method but that also didn't invoke the handleAll() method.

halfer
  • 19,824
  • 17
  • 99
  • 186
curious_brain
  • 391
  • 2
  • 17
  • I suggest you to remove `extends ResponseEntityExceptionHandler` this and try! I know you want it for `WebRequest`. Just comment them out and give a try – ray Sep 04 '21 at 02:32

3 Answers3

1

Your implementation is wrong, the handleException method will handle only those exceptions defined in the @ExceptionHandler annotation

@ExceptionHandler({
            HttpRequestMethodNotSupportedException.class,
            HttpMediaTypeNotSupportedException.class,
            HttpMediaTypeNotAcceptableException.class,
            MissingPathVariableException.class,
            MissingServletRequestParameterException.class,
            ServletRequestBindingException.class,
            ConversionNotSupportedException.class,
            TypeMismatchException.class,
            HttpMessageNotReadableException.class,
            HttpMessageNotWritableException.class,
            MethodArgumentNotValidException.class,
            MissingServletRequestPartException.class,
            BindException.class,
            NoHandlerFoundException.class,
            AsyncRequestTimeoutException.class
        })

so, these lines of code will always throw an exception.

try {
            tmpResponse = super.handleException(ex, request);
        } catch (Exception handlerEx) {
            logger.error(handlerEx.getMessage(), handlerEx);
        }

This idea is a mistake:

Since I am extending from ResourceEntityExceptionHandler, all the exceptions were being handled by handleException() method in that class.

red
  • 606
  • 10
  • 17
  • I agree with you. The statement you pointed out as mistake should have been worded: "```super.handlerException()``` call is sending all exceptions to ```ResponseEntityExceptionHandler.handleException()``` which re-throws any exception it is not meant to handle" So, the code segment ```tmpResponse = super.handleException(ex, request);``` actually gives me the ability to send custom exception to the caller of ReST endpoint no mater what the exception may be. – curious_brain Sep 10 '21 at 14:49
0

The ResponseEntityExceptionHandler handles a lot of exceptions so the handleAll will handle only the exceptions that are not included in ResponseEntityExceptionHandler, you can add a breakpoint in the handleException method inside ResponseEntityExceptionHandler to see if it is catching the exception. Another thing you can do is to throw a RuntimeException inside findAndSave and check if it is handle by handleAll

@ExceptionHandler({
            HttpRequestMethodNotSupportedException.class,
            HttpMediaTypeNotSupportedException.class,
            HttpMediaTypeNotAcceptableException.class,
            MissingPathVariableException.class,
            MissingServletRequestParameterException.class,
            ServletRequestBindingException.class,
            ConversionNotSupportedException.class,
            TypeMismatchException.class,
            HttpMessageNotReadableException.class,
            HttpMessageNotWritableException.class,
            MethodArgumentNotValidException.class,
            MissingServletRequestPartException.class,
            BindException.class,
            NoHandlerFoundException.class,
            AsyncRequestTimeoutException.class
        })
red
  • 606
  • 10
  • 17
0

I was able to solve it for myself. I minutely went over the details again that are posted here: Setting Precedence of Multiple @ControllerAdvice @ExceptionHandlers

This is how my @ControllerAdvice bean looks like now giving me that ability to stitch in a custom ApiResponse object for any exception that the Rest Controller throws.

@RestControllerAdvice
public class ErrorControllerAdvice extends ResponseEntityExceptionHandler {

    private static final Logger logger = LoggerFactory.getLogger(ErrorControllerAdvice.class);

    @ExceptionHandler({ ConstraintViolationException.class })
    public ResponseEntity<Object> handleConstraintViolationException(ConstraintViolationException ex, WebRequest request) {

        Metadata metadata = getRequestBody(request);

        ApiResponse apiResponse = new ApiResponse(); // details omitted

        return handleExceptionInternal(ex, apiResponse, new HttpHeaders(), apiResponse.getStatus(), request);
    }

    @ExceptionHandler({ Exception.class })
    public ResponseEntity<Object> handleAll(Exception ex, WebRequest request) {
        ResponseEntity<Object> tmpResponse = null;
        String message = null;

        try {
            tmpResponse = super.handleException(ex, request);
        } catch (Exception handlerEx) {
            logger.error(handlerEx.getMessage(), handlerEx);
        }

        HttpHeaders httpHeaders = tmpResponse == null ? new HttpHeaders() : tmpResponse.getHeaders();
        HttpStatus httpStatus = tmpResponse == null ? HttpStatus.INTERNAL_SERVER_ERROR : tmpResponse.getStatusCode();

        ApiResponse apiResponse = ApiResponse.builder()
                .timestamp(new Date())
                .status(httpStatus)
                .message(ex.getLocalizedMessage()) // ex and handlerEx will have same content because of line 98 in ResponseEntityExceptionHandler
                .path(((ServletWebRequest) request).getRequest().getRequestURI())
                .build();

        return new ResponseEntity<>(apiResponse, httpHeaders, httpStatus);
    }
}
curious_brain
  • 391
  • 2
  • 17
  • Please state the precise difference from the original advice class that solved the issue. It's not clear how this is an answer. – awgtek Sep 07 '21 at 19:16
  • Since I am extending from ```ResourceEntityExceptionHandler```, all the exceptions were being handled by ```handleException()``` method in that class. This method re```throw```s the exception if it's not already being handled. I was missing the ```try{}```-```catch{}``` block in my code that gave the ability to handle any exception thrown by Rest Controllers in my app. – curious_brain Sep 07 '21 at 20:03
  • The comment above by me is wrongly worded, as pointed out by @red. This is what I actually meant. ```super.handlerException()``` call is sending all exceptions to ```ResponseEntityExceptionHandler.handleException()``` which re-```throw```s any exception not meant to be handled by it. So, the code segment ```tmpResponse = super.handleException(ex, request);``` actually gives me the ability to send custom exception to the caller of ReST endpoint no mater what the exception may be. – curious_brain Sep 10 '21 at 14:51