SONAR-23079 Improve exception handling

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 13cacd3eefe7b2db9a359d3af154afea3a98cb31..1f7191db2579d9794ef6e508d5705b85143a34c6 100644 (file)
--- 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<RestError> 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<RestError> handleHttpMessageNotReadableException(HttpMessageNotReadableException httpMessageNotReadableException) {
-    String exceptionMessage = getExceptionMessage(httpMessageNotReadableException);
-    return new ResponseEntity<>(new RestError(exceptionMessage), HttpStatus.BAD_REQUEST);
+  protected ResponseEntity<RestError> 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<RestError> 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 15b5e75608c72340540d03db28a4ad692a1d69f1..ba469536e8ea65173f0ffd39f872f7d6ddb436b1 100644 (file)
--- 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<RestError> responseEntity = underTest.handleHttpMessageNotReadableException(exception);
+    ResponseEntity<RestError> 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<RestError> responseEntity = underTest.handleHttpMessageNotReadableException(exception);
-
-    assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
-    assertThat(responseEntity.getBody().message()).isEmpty();
+  private static Stream<Arguments> 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<RestError> responseEntity = underTest.handleHttpMessageNotReadableException(exception);
+    ResponseEntity<RestError> 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.");
   }
+
 }