Move NotificationClientException handling out of default.

src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java

@@ -20,7 +20,6 @@
 import uk.gov.hmcts.probate.model.ccd.ocr.ValidationResponse;
 import uk.gov.hmcts.probate.model.ccd.ocr.ValidationResponseStatus;
 import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse;
-import uk.gov.service.notify.NotificationClientException;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -109,16 +108,6 @@ public ResponseEntity<CallbackResponse> handle(RequestInformationParameterExcept
                 .body(errorResponse);
     }
 
-    @ExceptionHandler(value = NotificationClientException.class)
-    public ResponseEntity<ErrorResponse> handle(NotificationClientException exception) {
-        log.warn("Notification service exception", exception);
-        ErrorResponse errorResponse =
-            new ErrorResponse(SERVICE_UNAVAILABLE.value(), CLIENT_ERROR, exception.getMessage());
-        HttpHeaders headers = new HttpHeaders();
-        headers.setContentType(MediaType.APPLICATION_JSON);
-        return new ResponseEntity<>(errorResponse, headers, SERVICE_UNAVAILABLE);
-    }
-
     @ExceptionHandler(value = NotFoundException.class)
     public ResponseEntity<ErrorResponse> handle(NotFoundException exception) {
         log.warn("Not found exception", exception);

src/main/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandler.java

@@ -0,0 +1,81 @@
+package uk.gov.hmcts.probate.exception.handler;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.ControllerAdvice;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
+import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse;
+import uk.gov.service.notify.NotificationClientException;
+
+import java.util.ArrayList;
+import java.util.List;
+
+@Slf4j
+@ControllerAdvice
+public class NotificationClientExceptionHandler extends ResponseEntityExceptionHandler {
+    public static final String UNABLE_TO_SEND_EMAIL = "Unable to send email";
+
+    private final ObjectMapper objectMapper;
+
+    @Autowired
+    public NotificationClientExceptionHandler(ObjectMapper objectMapper) {
+        this.objectMapper = objectMapper;
+    }
+
+    @ExceptionHandler(value = NotificationClientException.class)
+    public ResponseEntity<CallbackResponse> handle(NotificationClientException exception) {
+        log.warn("Notification service exception", exception);
+        final List<String> errors = List.copyOf(getNotifyErrors(exception.getMessage()));
+
+        final CallbackResponse errorResponse = CallbackResponse.builder()
+                .errors(errors)
+                .build();
+        HttpHeaders headers = new HttpHeaders();
+        headers.setContentType(MediaType.APPLICATION_JSON);
+
+        return ResponseEntity.ok()
+                .headers(headers)
+                .body(errorResponse);
+    }
+
+    /**
+     * This relies on the behaviour of NotifyClient#performPostRequest passing the connection's error stream as a json
+     * string into the exception message, so is liable to break.
+     */
+    private List<String> getNotifyErrors(final String exMessage) {
+        final String exJson = exMessage.replaceFirst("[^{]*", "");
+
+        final List<String> errors = new ArrayList<>();
+        errors.add(UNABLE_TO_SEND_EMAIL);
+
+        final JsonNode outerJson;
+        try {
+            outerJson = objectMapper.readTree(exJson);
+        } catch (JsonProcessingException e) {
+            return errors;
+        }
+
+        if (!outerJson.isObject()) {
+            return errors;
+        }
+        if (!outerJson.has("errors") || !outerJson.get("errors").isArray()) {
+            return errors;
+        }
+
+        final JsonNode errorsJson = outerJson.get("errors");
+        for (final JsonNode errorJson : errorsJson) {
+            if (errorJson.isObject() && errorJson.has("message") && errorJson.get("message").isTextual()) {
+                final String message = errorJson.get("message").asText();
+                errors.add(message);
+            }
+        }
+        return errors;
+    }
+}

src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java

@@ -17,7 +17,6 @@
 import uk.gov.hmcts.probate.exception.model.FieldErrorResponse;
 import uk.gov.hmcts.probate.model.ccd.ocr.ValidationResponse;
 import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse;
-import uk.gov.service.notify.NotificationClientException;
 
 import java.util.Arrays;
 
@@ -43,9 +42,6 @@ class DefaultExceptionHandlerTest {
     @Mock
     private BadRequestException badRequestException;
 
-    @Mock
-    private NotificationClientException notificationClientException;
-
     @Mock
     private BusinessValidationException businessValidationException;
 
@@ -113,17 +109,6 @@ void shouldHandleMissingPDFDataAsStatusUN() {
         assertEquals(bve2Mock, response.getBody().getFieldErrors().get(1));
     }
 
-    @Test
-    void shouldReturnNotificationClientException() {
-        when(notificationClientException.getMessage()).thenReturn(EXCEPTION_MESSAGE);
-
-        ResponseEntity<ErrorResponse> response = underTest.handle(notificationClientException);
-
-        assertEquals(SERVICE_UNAVAILABLE, response.getStatusCode());
-        assertEquals(DefaultExceptionHandler.CLIENT_ERROR, response.getBody().getError());
-        assertEquals(EXCEPTION_MESSAGE, response.getBody().getMessage());
-    }
-
     @Test
     void shouldReturnBusinessValidationException() {
         when(businessValidationException.getUserMessage()).thenReturn(EXCEPTION_MESSAGE);

src/test/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandlerTest.java

@@ -0,0 +1,149 @@
+package uk.gov.hmcts.probate.exception.handler;
+
+import com.fasterxml.jackson.core.JsonParseException;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.http.ResponseEntity;
+import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse;
+import uk.gov.service.notify.NotificationClientException;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+import static org.springframework.http.HttpStatus.OK;
+
+public class NotificationClientExceptionHandlerTest {
+    NotificationClientExceptionHandler underTest;
+
+    ObjectMapper objectMapperSpy;
+
+    @BeforeEach
+    void setUp() {
+        objectMapperSpy = spy(ObjectMapper.class);
+        underTest = new NotificationClientExceptionHandler(objectMapperSpy);
+    }
+
+    @Test
+    void shouldReturnNotificationClientException() {
+        final String firstError = "first_error";
+        final String secondError = "second_error";
+        final String exceptMsg = new StringBuilder()
+                .append("Status code: 400 ")
+                .append("{\"errors\":[")
+                .append("{\"message\":\"").append(firstError).append("\"},")
+                .append("{\"message\":\"").append(secondError).append("\"}")
+                .append("]}")
+                .toString();
+
+        NotificationClientException notificationClientException = mock(NotificationClientException.class);
+        when(notificationClientException.getMessage()).thenReturn(exceptMsg);
+
+        ResponseEntity<CallbackResponse> response = underTest.handle(notificationClientException);
+
+        assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status");
+        assertEquals(3, response.getBody().getErrors().size(), "Expected three errors");
+        assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0),
+                "expected first error to be from handler");
+        assertEquals(firstError, response.getBody().getErrors().get(1),
+                "expected second error to match from exception message");
+        assertEquals(secondError, response.getBody().getErrors().get(2),
+                "expected third error to match from exception message");
+    }
+
+    @Test
+    void shouldReturnNotificationClientExceptionWhenInvalidJson() throws JsonProcessingException {
+        final String exceptMsg = "";
+
+        NotificationClientException notificationClientException = mock(NotificationClientException.class);
+        when(notificationClientException.getMessage()).thenReturn(exceptMsg);
+
+        when(objectMapperSpy.readTree(anyString())).thenThrow(new JsonParseException(exceptMsg));
+
+        ResponseEntity<CallbackResponse> response = underTest.handle(notificationClientException);
+
+        assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status");
+        assertEquals(1, response.getBody().getErrors().size(), "Expected only one error on json parse failure");
+        assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), "expected first error to be from handler");
+    }
+
+    @Test
+    void shouldReturnNotificationClientExceptionWhenJsonArray() throws JsonProcessingException {
+        final String exceptMsg = "[]";
+
+        NotificationClientException notificationClientException = mock(NotificationClientException.class);
+        when(notificationClientException.getMessage()).thenReturn(exceptMsg);
+
+        ResponseEntity<CallbackResponse> response = underTest.handle(notificationClientException);
+
+        assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status");
+        assertEquals(1, response.getBody().getErrors().size(), "Expected only one error on json parse failure");
+        assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), "expected first error to be from handler");
+    }
+
+    @Test
+    void shouldReturnNotificationClientExceptionWhenJsonMissingErrors() throws JsonProcessingException {
+        final String firstError = "first_error";
+        final String secondError = "second_error";
+        final String exceptMsg = "{}";
+
+        NotificationClientException notificationClientException = mock(NotificationClientException.class);
+        when(notificationClientException.getMessage()).thenReturn(exceptMsg);
+
+        ResponseEntity<CallbackResponse> response = underTest.handle(notificationClientException);
+
+        assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status");
+        assertEquals(1, response.getBody().getErrors().size(), "Expected only one error on json parse failure");
+        assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), "expected first error to be from handler");
+    }
+
+    @Test
+    void shouldReturnNotificationClientExceptionWhenJsonErrorsIsObject() throws JsonProcessingException {
+        final String firstError = "first_error";
+        final String secondError = "second_error";
+        final String exceptMsg = "{\"errors\":{}}";
+
+        NotificationClientException notificationClientException = mock(NotificationClientException.class);
+        when(notificationClientException.getMessage()).thenReturn(exceptMsg);
+
+        ResponseEntity<CallbackResponse> response = underTest.handle(notificationClientException);
+
+        assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status");
+        assertEquals(1, response.getBody().getErrors().size(), "Expected only one error on json parse failure");
+        assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), "expected first error to be from handler");
+    }
+
+    @Test
+    void shouldReturnNotificationClientExceptionWhenJsonErrorsContainsUnexpected() throws JsonProcessingException {
+        final String firstError = "first_error";
+        final String secondError = "second_error";
+        final String exceptMsg = new StringBuilder()
+                .append("{\"errors\":[")
+                .append("{\"message\":\"").append(firstError).append("\"},")
+                .append("[],")
+                .append("{},")
+                .append("{\"message\": 0},")
+                .append("{\"message\":\"").append(secondError).append("\"}")
+                .append("]}")
+                .toString();
+
+        NotificationClientException notificationClientException = mock(NotificationClientException.class);
+        when(notificationClientException.getMessage()).thenReturn(exceptMsg);
+
+        ResponseEntity<CallbackResponse> response = underTest.handle(notificationClientException);
+
+
+        assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status");
+        assertEquals(3, response.getBody().getErrors().size(), "Expected three errors");
+        assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0),
+                        "expected first error to be from handler");
+        assertEquals(firstError, response.getBody().getErrors().get(1),
+                        "expected second error to match from exception message");
+        assertEquals(secondError, response.getBody().getErrors().get(2),
+                        "expected third error to match from exception message");
+    }
+}