Po 691 api exception handling (#518)

* remove duplicate exception handling * fix column name in entity * change nocontent to notfound * remove unused test class * add auth error handling * add auth error handling * add more global exception handling * api unauthorised test * fix existing tests * add 503 test * change warn to error * remove test container * remove unused test * remove dissimilar assertion that is already tested in Response Util test * update exception http status and test * Remove I_AM_A_TEAPOT exceptions and improve error text generally * api unauthorised test * fix existing tests * add 503 test * remove test container * remove unused test * remove dissimilar assertion that is already tested in Response Util test * update exception http status and test * Remove I_AM_A_TEAPOT exceptions and improve error text generally * merge conflicts * merge conflicts * merge conflicts * more coverage * checkstyle
hmcts · Sep 5, 2024 · c9526d2 · c9526d2
1 parent 8fce388
commit c9526d2
Show file tree

Hide file tree

Showing 5 changed files with 201 additions and 139 deletions.
diff --git a/src/main/java/uk/gov/hmcts/opal/authentication/exception/AuthenticationError.java b/src/main/java/uk/gov/hmcts/opal/authentication/exception/AuthenticationError.java
@@ -11,19 +11,19 @@ public enum AuthenticationError implements OpalApiError {
 
     FAILED_TO_OBTAIN_ACCESS_TOKEN(
         "100",
-        HttpStatus.INTERNAL_SERVER_ERROR,
+        HttpStatus.UNAUTHORIZED,
         "Failed to obtain access token"
     ),
 
     FAILED_TO_VALIDATE_ACCESS_TOKEN(
         "101",
-        HttpStatus.INTERNAL_SERVER_ERROR,
+        HttpStatus.UNAUTHORIZED,
         "Failed to validate access token"
     ),
 
     FAILED_TO_PARSE_ACCESS_TOKEN(
         "102",
-        HttpStatus.INTERNAL_SERVER_ERROR,
+        HttpStatus.UNAUTHORIZED,
         "Failed to parse access token"
     ),
 

diff --git a/src/main/java/uk/gov/hmcts/opal/authentication/exception/CustomAuthenticationExceptions.java b/src/main/java/uk/gov/hmcts/opal/authentication/exception/CustomAuthenticationExceptions.java
@@ -30,7 +30,7 @@ public void commence(HttpServletRequest request,
         // Write the custom message to the response body
         try (PrintWriter writer = response.getWriter()) {
             writer.write("{\"error\": \"Unauthorized\", \"message\":"
-                             + " \"Unauthorized: request could not be authorized\"}");
+                             + authException.getMessage() + "}");
         }
     }
 

diff --git a/src/main/java/uk/gov/hmcts/opal/controllers/advice/GlobalExceptionHandler.java b/src/main/java/uk/gov/hmcts/opal/controllers/advice/GlobalExceptionHandler.java
@@ -13,11 +13,13 @@
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.dao.InvalidDataAccessResourceUsageException;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.transaction.TransactionSystemException;
 import org.springframework.web.HttpMediaTypeNotAcceptableException;
+import org.springframework.web.HttpMediaTypeNotSupportedException;
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import uk.gov.hmcts.opal.authentication.exception.MissingRequestHeaderException;
@@ -26,6 +28,7 @@
 import uk.gov.hmcts.opal.exception.OpalApiException;
 import uk.gov.hmcts.opal.launchdarkly.FeatureDisabledException;
 
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 import static uk.gov.hmcts.opal.authentication.service.AccessTokenService.AUTH_HEADER;
@@ -36,28 +39,36 @@
 @RequiredArgsConstructor
 public class GlobalExceptionHandler {
 
-    public static final String ERROR_MESSAGE = "errorMessage";
+    public static final String ERROR = "error";
+
+    public static final String MESSAGE = "message";
+
+    public static final String DB_UNAVAILABLE_MESSAGE = "Opal Fines Database is currently unavailable";
+
 
     private final AccessTokenService tokenService;
 
     @ExceptionHandler(FeatureDisabledException.class)
     public ResponseEntity<String> handleFeatureDisabledException(FeatureDisabledException ex) {
-        return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body(ex.getMessage());
+        return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED)
+            .contentType(MediaType.APPLICATION_JSON).body(ex.getMessage());
     }
 
     @ExceptionHandler(MissingRequestHeaderException.class)
     public ResponseEntity<String> handleMissingRequestHeaderException(MissingRequestHeaderException ex) {
-        return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ex.getMessage());
+        return ResponseEntity.status(HttpStatus.BAD_REQUEST)
+            .contentType(MediaType.APPLICATION_JSON).body(ex.getMessage());
     }
 
     @ExceptionHandler({PermissionNotAllowedException.class, AccessDeniedException.class})
     public ResponseEntity<String> handlePermissionNotAllowedException(Exception ex,
                                                                       HttpServletRequest request) {
         String authorization = request.getHeader(AUTH_HEADER);
         String preferredName = extractPreferredUsername(authorization, tokenService);
-        String message = String.format("For user %s, %s", preferredName, ex.getMessage());
+        String message = String.format("{\"error\": \"Forbidden\", \"message\" : \"For user %s, %s \"}", preferredName,
+                                       ex.getMessage());
         log.error(message);
-        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(message);
+        return ResponseEntity.status(HttpStatus.FORBIDDEN).contentType(MediaType.APPLICATION_JSON).body(message);
     }
 
     @ExceptionHandler(HttpMediaTypeNotAcceptableException.class)
@@ -67,44 +78,50 @@ public ResponseEntity<Map<String, String>> handleHttpMediaTypeNotAcceptableExcep
         log.error(":handleHttpMediaTypeNotAcceptableException: {}", ex.getMessage());
         log.error(":handleHttpMediaTypeNotAcceptableException:", ex.getCause());
 
-        Map<String, String> body = Map.of(
-            "error", "Not Acceptable",
-            "message", "The server cannot produce a response matching the request Accept header"
-        );
-        return ResponseEntity.status(HttpStatus.NOT_ACCEPTABLE).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, "Not Acceptable");
+        body.put(MESSAGE, ex.getMessage() + ", " + ex.getBody().getDetail());
+
+        return ResponseEntity.status(HttpStatus.NOT_ACCEPTABLE).contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler
     public ResponseEntity<Map<String, String>> handlePropertyValueException(PropertyValueException pve) {
         log.error(":handlePropertyValueException: {}", pve.getMessage());
-        Map<String, String> body = Map.of(
-            ERROR_MESSAGE, pve.getMessage(),
-            "entity", pve.getEntityName(),
-            "property", pve.getPropertyName()
-        );
-        return ResponseEntity.status(HttpStatus.I_AM_A_TEAPOT).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, pve.getMessage());
+        body.put("entity", pve.getEntityName());
+        body.put("property", pve.getPropertyName());
+
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
-    @ExceptionHandler
+    @ExceptionHandler({HttpMediaTypeNotSupportedException.class, HttpMessageNotReadableException.class})
     public ResponseEntity<Map<String, String>> handleHttpMessageNotReadableException(
-        HttpMessageNotReadableException hmnre) {
+        Exception ex) {
+
+        log.error(":handleHttpMessageNotReadableException: {}", ex.getMessage());
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, ex.getMessage());
+        body.put(MESSAGE,
+            "The request body could not be read, ensure content-type is application/json");
 
-        log.error(":handleHttpMessageNotReadableException: {}", hmnre.getMessage());
-        Map<String, String> body = Map.of(
-            ERROR_MESSAGE, hmnre.getMessage()
-        );
-        return ResponseEntity.status(HttpStatus.I_AM_A_TEAPOT).body(body);
+        return ResponseEntity.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE)
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler
     public ResponseEntity<Map<String, String>> handleInvalidDataAccessApiUsageException(
         InvalidDataAccessApiUsageException idaaue) {
 
         log.error(":handleInvalidDataAccessApiUsageException: {}", idaaue.getMessage());
-        Map<String, String> body = Map.of(
-            ERROR_MESSAGE, idaaue.getMessage()
-        );
-        return ResponseEntity.status(HttpStatus.I_AM_A_TEAPOT).body(body);
+
+        Map<String, String> body = new LinkedHashMap<>();
+
+        body.put(ERROR, idaaue.getMessage());
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler
@@ -114,10 +131,11 @@ public ResponseEntity<Map<String, String>> handleInvalidDataAccessResourceUsageE
         log.error(":handleInvalidDataAccessApiUsageException: {}", idarue.getMessage());
         log.error(":handleInvalidDataAccessApiUsageException:", idarue.getRootCause());
 
-        Map<String, String> body = Map.of(
-            ERROR_MESSAGE, idarue.getMessage()
-        );
-        return ResponseEntity.status(HttpStatus.I_AM_A_TEAPOT).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, idarue.getMessage());
+
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler
@@ -127,10 +145,10 @@ public ResponseEntity<Map<String, String>> handleEntityNotFoundException(
         log.error(":handleEntityNotFoundException: {}", entityNotFoundException.getMessage());
         log.error(":handleEntityNotFoundException:", entityNotFoundException.getCause());
 
-        Map<String, String> body = Map.of(
-            ERROR_MESSAGE, entityNotFoundException.getMessage()
-        );
-        return ResponseEntity.status(HttpStatus.NOT_FOUND).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, "Entity Not Found");
+        body.put(MESSAGE, entityNotFoundException.getMessage());
+        return ResponseEntity.status(HttpStatus.NOT_FOUND).contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler(OpalApiException.class)
@@ -140,11 +158,11 @@ public ResponseEntity<Map<String, String>> handleOpalApiException(
         log.error(":handleOpalApiException: {}", opalApiException.getMessage());
         log.error(":handleOpalApiException:", opalApiException.getCause());
 
-        Map<String, String> body = Map.of(
-            "error", opalApiException.getError().getHttpStatus().getReasonPhrase(),
-            "message", opalApiException.getMessage()
-        );
-        return ResponseEntity.status(opalApiException.getError().getHttpStatus()).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, opalApiException.getError().getHttpStatus().getReasonPhrase());
+        body.put(MESSAGE, opalApiException.getMessage());
+        return ResponseEntity.status(opalApiException.getError().getHttpStatus())
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler({ServletException.class, TransactionSystemException.class, PersistenceException.class})
@@ -153,19 +171,18 @@ public ResponseEntity<Map<String, String>> handleDatabaseExceptions(Exception ex
         if (ex instanceof QueryTimeoutException) {
             log.error(":handleQueryTimeoutException: {}", ex.getMessage());
 
-            Map<String, String> body = Map.of(
-                "error", "Request Timeout",
-                "message", "The request did not receive a response from the database within the timeout period"
-            );
-            return ResponseEntity.status(HttpStatus.REQUEST_TIMEOUT).body(body);
+            Map<String, String> body = new LinkedHashMap<>();
+            body.put(ERROR, "Request Timeout");
+            body.put(MESSAGE, "The request did not receive a response from the database within the timeout period");
+            return ResponseEntity.status(HttpStatus.REQUEST_TIMEOUT).contentType(MediaType.APPLICATION_JSON).body(body);
         }
 
         // If it's not a QueryTimeoutException, return a generic internal server error
-        Map<String, String> body = Map.of(
-            "error", "Internal Server Error",
-            "message", "An unexpected error occurred"
-        );
-        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, "Internal Server Error");
+        body.put(MESSAGE, "An unexpected error occurred");
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler
@@ -176,17 +193,19 @@ public ResponseEntity<Map<String, String>> handlePsqlException(
         log.error(":handlePSQLException:", psqlException.getCause());
 
         if (psqlException.getCause() instanceof java.net.ConnectException) {
-            Map<String, String> body = Map.of(
-                "error", "Service Unavailable", "message",
-                "Opal Fines Database is currently unavailable"
-            );
-            return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(body);
+            Map<String, String> body = new LinkedHashMap<>();
+            body.put(ERROR, "Service Unavailable");
+            body.put(MESSAGE, DB_UNAVAILABLE_MESSAGE);
+            return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE)
+                .contentType(MediaType.APPLICATION_JSON).body(body);
         }
 
-        Map<String, String> body = Map.of(
-            "error", "Internal Server Error", "message", psqlException.getMessage()
-        );
-        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, "Internal Server Error");
+        body.put(MESSAGE, psqlException.getMessage());
+
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .contentType(MediaType.APPLICATION_JSON).body(body);
     }
 
     @ExceptionHandler
@@ -196,9 +215,11 @@ public ResponseEntity<Map<String, String>> handleDataAccessResourceFailureExcept
         log.error(":handleDataAccessResourceFailureException: {}", dataAccessResourceFailureException.getMessage());
         log.error(":handleDataAccessResourceFailureException:", dataAccessResourceFailureException.getCause());
 
-        Map<String, String> body = Map.of(
-            "error", "Service Unavailable", "message", "Opal Fines Database is currently unavailable"
-        );
-        return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(body);
+        Map<String, String> body = new LinkedHashMap<>();
+        body.put(ERROR, "Service Unavailable");
+        body.put(MESSAGE, DB_UNAVAILABLE_MESSAGE);
+        return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).contentType(MediaType.APPLICATION_JSON).body(body);
     }
+
+
 }
diff --git a/...t/java/uk/gov/hmcts/opal/authentication/exception/CustomAuthenticationExceptionsTest.java b/...t/java/uk/gov/hmcts/opal/authentication/exception/CustomAuthenticationExceptionsTest.java
@@ -39,8 +39,8 @@ void commenceShouldReturnUnauthorizedResponse() throws IOException, ServletExcep
 
         verify(response).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
         verify(response).setContentType("application/json");
-        verify(writer).write("{\"error\": \"Unauthorized\", \"message\": "
-                                 + "\"Unauthorized: request could not be authorized\"}");
+        verify(writer).write("{\"error\": \"Unauthorized\", \"message\":"
+                                 + authException.getMessage() + "}");
     }
 
     @Test