Add special handling of disconnected client errors in ExceptionHandlerExceptionResolver #26181
Comments
|
Thanks for the bug report. I think this is something that the Spring Framework team will need to consider. I'll transfer the issue for them to review. |
spring-projects-issues
added
the
status: waiting-for-triage
|
Thanks for the sample. At the point of handling the controller method return value, the exception is allowed to propagate as it may be handled for example by an It's a little different at the point of Furthermore, many times the exact opposite behavior is frequently requested, i.e. not to log I/O exceptions for clients that have gone away, especially for long running connections. We have a couple of places in the code (SockJS and WebFlux) where You can add an |
rstoyanchev
added
status: waiting-for-feedback
|
Thanks for your prompt response @rstoyanchev, appreciated.
I could not follow you on how to handle the exception thrown while Spring tries to transmit the response of a successfully completed exception handler method call. I have added the following simple exception handler to the snippet I shared above: @ExceptionHandler(ClientAbortException.class)
void handleClientAbortException() {}And you might guess that I still get the very same stack trace in the logs. Given the exception is not thrown by the code I have in my controller, but Spring itself, I am clueless about how to suppress it. Do I miss your point here? Would you mind elaborating on how can I "add an @ExceptionHandler and use that to log as needed", please? |
spring-projects-issues
added
status: feedback-provided
|
I'm not sure why you are not able to capture it. All I did was literally to add the @RestController
public static class MainController {
private static final String RESPONSE_PAYLOAD;
static {
StringBuilder stringBuilder = new StringBuilder();
for (int i = 0; i < 1_000_000; i++) {
stringBuilder.append("foo bar baz ").append(i);
}
RESPONSE_PAYLOAD = stringBuilder.toString();
}
// ...
@ExceptionHandler
public void handleException(IOException ex) {
System.out.println("IOException: " + ex);
}
// ...
}I wrote it slightly differently but all other variations work too: @ExceptionHandler
public void handle(ClientAbortException ex) { }
@ExceptionHandler(ClientAbortException.class)
public void handle() { }
@ExceptionHandler(IOException.class)
public void handle() { } |
rstoyanchev
added
status: waiting-for-feedback
|
@rstoyanchev, please see the following test where import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import org.apache.catalina.connector.ClientAbortException;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.web.server.LocalServerPort;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URI;
@SpringBootTest(
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
classes = MainApplicationTest.TestApplication.class)
class MainApplicationTest {
private static final Logger LOGGER =
LoggerFactory.getLogger(MainApplicationTest.class);
@LocalServerPort
private int port;
@Test
void testWork() throws Exception {
withLogVerification(() -> query("/work"));
}
@Test
void testCollapse() throws Exception {
withLogVerification(() -> query("/collapse"));
}
@FunctionalInterface
private interface ThrowingRunnable {
void run() throws Exception;
}
private static void withLogVerification(
ThrowingRunnable body
) throws Exception {
ListAppender<ILoggingEvent> appender = captureLogs();
body.run();
verifyLogs(appender);
}
private static ListAppender<ILoggingEvent> captureLogs() {
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.list.clear();
appender.setContext((LoggerContext) LoggerFactory.getILoggerFactory());
ch.qos.logback.classic.Logger logger =
(ch.qos.logback.classic.Logger) LoggerFactory
.getLogger(Logger.ROOT_LOGGER_NAME);
logger.addAppender(appender);
appender.start();
return appender;
}
private static void verifyLogs(
ListAppender<ILoggingEvent> appender
) throws InterruptedException {
// It takes a while for Spring to dump the ERROR logs, hence waiting.
Thread.sleep(1_000);
Assertions
.assertThat(appender.list)
.noneSatisfy(event -> Assertions
.assertThat(event
.getLevel()
.isGreaterOrEqual(Level.ERROR))
.isTrue());
}
private void query(String path) throws IOException {
HttpURLConnection urlConnection = (HttpURLConnection) URI
.create("http://localhost:" + port + path)
.toURL()
.openConnection();
try {
// Prematurely close the connection.
urlConnection.getInputStream().close();
int responseCode = urlConnection.getResponseCode();
LOGGER.info("{} -> {}", path, responseCode);
} finally {
urlConnection.disconnect();
}
}
@SpringBootApplication
static class TestApplication {
@RestController
static class TestController {
private static final String RESPONSE_PAYLOAD;
static {
StringBuilder stringBuilder = new StringBuilder();
for (int i = 0; i < 1_000_000; i++) {
stringBuilder.append("foo bar baz ").append(i);
}
RESPONSE_PAYLOAD = stringBuilder.toString();
}
private static final class CustomError extends RuntimeException {}
@GetMapping("/collapse")
public String collapse() {
throw new CustomError();
}
@ExceptionHandler(CustomError.class)
public String handleCustomError() {
return RESPONSE_PAYLOAD;
}
@ExceptionHandler(ClientAbortException.class)
public void handleClientAbortException() {}
@GetMapping("/work")
public String work() {
return RESPONSE_PAYLOAD;
}
}
}
} |
spring-projects-issues
added
status: feedback-provided
|
@vy thanks for the extra sample but I can't debug that at the moment. I can assure you there is no reason why an For further debugging, I suggest you put an exception breakpoint along with a breakpoint in |
rstoyanchev
added
status: waiting-for-feedback
A couple of my team mates were able to reproduce this anomaly using
I will see how far I can go, thanks for the tip. In the meantime, I will really appreciate if you can confirm that the above shared standalone test also fails for you. That will at least put us on the same page. |
spring-projects-issues
added
status: feedback-provided
|
I'm running into a very similar issue. My server makes requests to other services, which time out regularly. The timeout exceptions are caught by an exception handler. However, the client may have already aborted by the time the timeout occurs. This leads to an uncaught ClientAbortedException in the exception handler. Here's a very minimal reproduction path. Run the following application: Make a request with a timeout: This results in a similar server error as the original example. |
|
Is there any update on this issue @rstoyanchev? |
|
It's even weirder for me. If I run a second request before the first one has finished I get duplicate information in the second one that came in from the first one even though it finished with a ClientAbortedException |
rstoyanchev
added
type: enhancement
|
We have logic in a couple of places to recognize server specific IO exceptions that indicate the client went away, and to apply special logging at DEBUG and TRACE only, like here. We can apply similar in |
rstoyanchev
changed the title
|
Just a quick note about a small further refinement in #32359. |
Using Spring Boot 2.4.0 and Web MVC, when a client closes the connection before the server completes the response transmission, regular request handlers (e.g.,
@GetMapping) don't log any failures regarding the raisedClientAbortExceptionby Tomcat. Whereas, in the case of a@ExceptionHandler, aClientAbortExceptionis logged. I think, the very sameClientAbortExceptionneeds to be suppressed, as is the case in regular request handlers.Consider the following simple application:
Note that both handlers (i.e., request and exception) return back the very same payload.
Below I use toxiproxy to simulate the premature connection reset:
If I query
http://localhost:18080/work, nothing gets logged to the server console. Whereas, queryinghttp://localhost:18080/collapsecauses the following failure in the server:The text was updated successfully, but these errors were encountered: