Stop reporting a client that went away as a server failure
The editor polls a running execution and drops the request when it navigates, refreshes or supersedes it - routine, and more likely the longer the response takes to write, which an execution view carrying a large global input does. Each one was logged as "Unhandled request failure" with a hundred-line stack trace, for something nobody can act on and where the reply goes to a connection that is already gone. Recognised through the cause chain, because Jackson wraps the broken pipe several times over before it surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
97f3ab0e66
commit
337b2939ac
|
|
@ -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.
|
||||
*
|
||||
* <p>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()
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue