From 85c9dd709f6ab07f30b00f85f0fb178d63f9a166 Mon Sep 17 00:00:00 2001 From: Julien Camus Date: Wed, 18 Sep 2024 18:06:53 +0200 Subject: [PATCH] SONAR-23079 Improve exception handling --- .../RestResponseEntityExceptionHandler.java | 33 +++++++---- ...estResponseEntityExceptionHandlerTest.java | 59 ++++++++++--------- 2 files changed, 53 insertions(+), 39 deletions(-) diff --git a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java index 13cacd3eefe..1f7191db257 100644 --- a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java +++ b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java @@ -19,9 +19,10 @@ */ package org.sonar.server.v2.common; -import com.fasterxml.jackson.databind.exc.InvalidFormatException; import java.util.Optional; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; @@ -33,13 +34,19 @@ import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.validation.BindException; import org.springframework.validation.FieldError; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.servlet.NoHandlerFoundException; @RestControllerAdvice public class RestResponseEntityExceptionHandler { + private static final Logger LOGGER = LoggerFactory.getLogger(RestResponseEntityExceptionHandler.class); + @ExceptionHandler({IllegalStateException.class}) @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) protected ResponseEntity handleIllegalStateException(IllegalStateException illegalStateException) { @@ -80,18 +87,24 @@ public class RestResponseEntityExceptionHandler { return new ResponseEntity<>(new RestError(notFoundException.getMessage()), HttpStatus.NOT_FOUND); } - @ExceptionHandler({HttpMessageNotReadableException.class}) + @ExceptionHandler({ + HttpMessageNotReadableException.class, + MethodArgumentTypeMismatchException.class, + HttpRequestMethodNotSupportedException.class, + ServletRequestBindingException.class, + NoHandlerFoundException.class + }) @ResponseStatus(HttpStatus.BAD_REQUEST) - protected ResponseEntity handleHttpMessageNotReadableException(HttpMessageNotReadableException httpMessageNotReadableException) { - String exceptionMessage = getExceptionMessage(httpMessageNotReadableException); - return new ResponseEntity<>(new RestError(exceptionMessage), HttpStatus.BAD_REQUEST); + protected ResponseEntity handleClientSideException(Exception exception) { + LOGGER.error("Error processing request", exception); + return new ResponseEntity<>(new RestError("An error occurred while processing the request."), HttpStatus.BAD_REQUEST); } - private static String getExceptionMessage(HttpMessageNotReadableException httpMessageNotReadableException) { - if (httpMessageNotReadableException.getRootCause() instanceof InvalidFormatException invalidFormatException) { - return invalidFormatException.getOriginalMessage(); - } - return Optional.ofNullable(httpMessageNotReadableException.getMessage()).orElse(""); + @ExceptionHandler({Exception.class}) + @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) + protected ResponseEntity handleUnhandledException(Exception exception) { + LOGGER.error("Unhandled exception", exception); + return new ResponseEntity<>(new RestError("An unexpected error occurred. Please contact support if the problem persists."), HttpStatus.INTERNAL_SERVER_ERROR); } } diff --git a/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java b/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java index 15b5e75608c..ba469536e8e 100644 --- a/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java +++ b/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java @@ -19,54 +19,55 @@ */ package org.sonar.server.v2.common; -import com.fasterxml.jackson.databind.exc.InvalidFormatException; -import org.junit.Test; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.sonar.server.v2.api.model.RestError; +import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.ServletRequestBindingException; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.servlet.NoHandlerFoundException; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -public class RestResponseEntityExceptionHandlerTest { +class RestResponseEntityExceptionHandlerTest { - private RestResponseEntityExceptionHandler underTest = new RestResponseEntityExceptionHandler(); + private final RestResponseEntityExceptionHandler underTest = new RestResponseEntityExceptionHandler(); - @Test - public void handleHttpMessageNotReadableException_whenCauseIsNotInvalidFormatException_shouldUseMessage() { - - HttpMessageNotReadableException exception = new HttpMessageNotReadableException("Message not readable", new Exception()); + @ParameterizedTest + @MethodSource("clientSideExceptionsProvider") + void handleClientSideException_shouldUseGenericMessage(Exception exception) { - ResponseEntity responseEntity = underTest.handleHttpMessageNotReadableException(exception); + ResponseEntity responseEntity = underTest.handleClientSideException(exception); assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(responseEntity.getBody().message()).isEqualTo("Message not readable; nested exception is java.lang.Exception"); + assertThat(responseEntity.getBody().message()).isEqualTo("An error occurred while processing the request."); } - @Test - public void handleHttpMessageNotReadableException_whenCauseIsNotInvalidFormatExceptionAndMessageIsNull_shouldUseEmptyStringAsMessage() { - - HttpMessageNotReadableException exception = new HttpMessageNotReadableException(null, (Exception) null); - - ResponseEntity responseEntity = underTest.handleHttpMessageNotReadableException(exception); - - assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(responseEntity.getBody().message()).isEmpty(); + private static Stream clientSideExceptionsProvider() { + return Stream.of( + Arguments.of(new HttpMessageNotReadableException("", (HttpInputMessage) null)), + Arguments.of(new MethodArgumentTypeMismatchException(null, null, null, null, null)), + Arguments.of(new HttpRequestMethodNotSupportedException("")), + Arguments.of(new ServletRequestBindingException("")), + Arguments.of(new NoHandlerFoundException("", null, null))); } @Test - public void handleHttpMessageNotReadableException_whenCauseIsInvalidFormatException_shouldUseMessageFromCause() { - - InvalidFormatException cause = mock(InvalidFormatException.class); - when(cause.getOriginalMessage()).thenReturn("Cause message"); + void handleUnhandledException_shouldUseGenericMessage() { - HttpMessageNotReadableException exception = new HttpMessageNotReadableException("Message not readable", cause); + Exception exception = new Exception(); - ResponseEntity responseEntity = underTest.handleHttpMessageNotReadableException(exception); + ResponseEntity responseEntity = underTest.handleUnhandledException(exception); - assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(responseEntity.getBody().message()).isEqualTo("Cause message"); + assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(responseEntity.getBody().message()).isEqualTo("An unexpected error occurred. Please contact support if the problem persists."); } + } -- 2.39.5