diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java index 4b4a684..8d3fc6e 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java @@ -15,6 +15,7 @@ import org.springframework.validation.ObjectError; import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.request.async.AsyncRequestNotUsableException; import org.springframework.web.server.ResponseStatusException; import jakarta.servlet.http.HttpServletRequest; @@ -78,8 +79,27 @@ public class ApiExceptionHandler { return problem; } + /** + * A client that went away mid-response, which is not a failure of this server. + * + *
The browser polls a running execution and drops the request when it navigates, refreshes, + * or simply supersedes it - and an execution view carrying a large global input takes long + * enough to write for that to be routine. Reported as an unhandled failure it produced a + * hundred-line stack trace, in the error log, for something nobody can act on and nothing can + * be returned to: the connection is already gone, so the ProblemDetail below is written to + * no one. + */ + @ExceptionHandler(AsyncRequestNotUsableException.class) + public void handleClientGoneAway(AsyncRequestNotUsableException e) { + logger.debug("Client disconnected before the response was written: {}", e.getMessage()); + } + @ExceptionHandler(Exception.class) public ProblemDetail handleGenericException(Exception e) { + if (isClientDisconnect(e)) { + logger.debug("Client disconnected before the response was written: {}", e.getMessage()); + return null; + } logger.error("Unhandled request failure", e); ProblemDetail problem = ProblemDetail.forStatus(HttpStatus.INTERNAL_SERVER_ERROR); problem.setTitle(HttpStatus.INTERNAL_SERVER_ERROR.toString()); @@ -87,6 +107,28 @@ public class ApiExceptionHandler { return problem; } + /** + * Whether a failure is only the client having gone away. Jackson wraps the broken pipe several + * times over - a write failure surfaces as a databind error inside a converter error - so the + * cause chain is what has to be asked, not the type on top. + */ + private boolean isClientDisconnect(Throwable error) { + for (Throwable current = error; current != null; current = current.getCause()) { + if (current instanceof AsyncRequestNotUsableException) { + return true; + } + if (current instanceof java.io.IOException + && current.getMessage() != null + && current.getMessage().contains("Broken pipe")) { + return true; + } + if (current.getCause() == current) { + break; + } + } + return false; + } + private String resolveReadableMessage(HttpMessageNotReadableException e) { Throwable cause = e.getMostSpecificCause(); String message = cause != null && cause.getMessage() != null && !cause.getMessage().isBlank() diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandlerClientDisconnectTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandlerClientDisconnectTest.java new file mode 100644 index 0000000..3e4946f --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandlerClientDisconnectTest.java @@ -0,0 +1,38 @@ +package it.cnr.isti.workflow.manager.controllers; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.io.IOException; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; +import org.springframework.http.ProblemDetail; +import org.springframework.http.converter.HttpMessageNotWritableException; + +class ApiExceptionHandlerClientDisconnectTest { + + @Test + void saysNothingWhenTheClientWentAwayMidResponse() { + // The browser polls a running execution and drops the request when it navigates or + // supersedes it. Reported as an unhandled failure this filled the error log with a stack + // trace for something nobody can act on, and the reply goes to a connection already gone. + ApiExceptionHandler handler = new ApiExceptionHandler(); + HttpMessageNotWritableException brokenPipe = new HttpMessageNotWritableException( + "Could not write JSON", new IllegalStateException(new IOException("Broken pipe"))); + + assertNull(handler.handleGenericException(brokenPipe)); + } + + @Test + void stillReportsAFailureThatIsThisServersOwn() { + ApiExceptionHandler handler = new ApiExceptionHandler(); + + ProblemDetail problem = handler.handleGenericException(new IllegalStateException("something broke")); + + assertNotNull(problem); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR.value(), problem.getStatus()); + assertEquals("something broke", problem.getDetail()); + } +}