diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapper.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapper.java index 505bf9678..486853b9f 100644 --- a/rest/src/main/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapper.java +++ b/rest/src/main/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapper.java @@ -15,7 +15,6 @@ import java.util.HashMap; import java.util.Map; -import org.apache.catalina.connector.ClientAbortException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.core.annotation.Order; @@ -36,6 +35,7 @@ import jakarta.servlet.http.HttpServletRequest; @RestControllerAdvice +@Order(ValidationExceptionMapper.ORDER + 1) public class ApplicationExceptionMapper { private static final Logger LOGGER = LoggerFactory.getLogger(ApplicationExceptionMapper.class); @@ -53,16 +53,7 @@ public class ApplicationExceptionMapper { EXCEPTION_STATUS_MAP.put(NoResourceFoundException.class, HttpStatus.NOT_FOUND); } - @ExceptionHandler(ClientAbortException.class) - @Order(ValidationExceptionMapper.ORDER + 1) - public void handleClientAborted( - HttpServletRequest request, ClientAbortException clientAbortException) { - logException( - request, clientAbortException); // socket closed, cannot return any error response - } - @ExceptionHandler(Throwable.class) - @Order(ValidationExceptionMapper.ORDER + 2) public ResponseEntity handleAll(HttpServletRequest request, Throwable th) { logException(request, th); diff --git a/rest/src/test/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapperTest.java b/rest/src/test/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapperTest.java new file mode 100644 index 000000000..c800d5b3f --- /dev/null +++ b/rest/src/test/java/com/netflix/conductor/rest/controllers/ApplicationExceptionMapperTest.java @@ -0,0 +1,90 @@ +/* + * Copyright 2024 Conductor Authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package com.netflix.conductor.rest.controllers; + +import java.util.Collections; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; + +import com.netflix.conductor.model.TaskModel; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import static org.mockito.Mockito.*; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +public class ApplicationExceptionMapperTest { + + private QueueAdminResource queueAdminResource; + + private MockMvc mockMvc; + + private static MockedStatic mockLoggerFactory; + private static final Logger logger = mock(Logger.class); + + @Before + public void before() { + mockLoggerFactory = Mockito.mockStatic(LoggerFactory.class); + when(LoggerFactory.getLogger(ApplicationExceptionMapper.class)).thenReturn(logger); + + this.queueAdminResource = mock(QueueAdminResource.class); + this.mockMvc = + MockMvcBuilders.standaloneSetup(this.queueAdminResource) + .setControllerAdvice(new ApplicationExceptionMapper()) + .build(); + } + + @After + public void after() { + mockLoggerFactory.close(); + } + + @Test + public void testException() throws Exception { + var exception = new Exception(); + // pick a method that raises a generic exception + doThrow(exception).when(this.queueAdminResource).update(any(), any(), any(), any()); + + // verify we do send an error response + this.mockMvc + .perform( + MockMvcRequestBuilders.post( + "/api/queue/update/workflowId/taskRefName/{status}", + TaskModel.Status.SKIPPED) + .contentType(MediaType.APPLICATION_JSON) + .content( + new ObjectMapper() + .writeValueAsString(Collections.emptyMap()))) + .andDo(print()) + .andExpect(status().is5xxServerError()); + // verify the error was logged + verify(logger) + .error( + "Error {} url: '{}'", + "Exception", + "/api/queue/update/workflowId/taskRefName/SKIPPED", + exception); + verifyNoMoreInteractions(logger); + } +}