I stumbled upon https://github.com/spring-projects/spring-framework/issues/34264 which caused some bigger issues internally because some server side connection resets resulted in a 200 successful response. While the biggest issue was solved I think that it is quite unsettling that an exception results in a 200 response and this is still the case for the current spring version (6.2.3) shipped with spring boot 3.4.3.
Minimal example which returns a 200 even though an exception is thrown and not properly handled:
@RestController
@ControllerAdvice
public class DemoController {
private static final Logger log = LoggerFactory.getLogger(DemoController.class);
@GetMapping
public String demo() {
throw new RuntimeException("connection reset by peer");
}
@ExceptionHandler(RuntimeException.class)
public ResponseEntity<Object> handleRuntimeException(RuntimeException e) {
log.error(e.getMessage());
throw e;
}
}
While this usecase might not look to useful we definitely have use cases where we want to handle an exception in some custom way using an exception handler but only in specific cases. So something like this:
@ExceptionHandler(RuntimeException.class)
public ResponseEntity<Object> handleRuntimeException(RuntimeException e) {
if (matchesSomeCriteria(e)) {
return specialHandling(e);
}
throw e;
}
While investigating I found that this is probably caused here: https://github.com/spring-projects/spring-framework/blob/75329e6d9d93180d9edceb81838b0d96a145d665/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java#L463-L465
If I understand this correctly the idea is to not continue handling exceptions in case the client closed the connection. Since the check for this is just using some string comparison on the exception message it is quite easy to have an exception matching this as can be seen above.
If no one has ideas on how to fix this properly I would suggest to at least set a 500 error status on the ModelView that is created in the ExceptionHandlerExceptionResolver to make this a little less worrying. While the behaviour might still be unexpected it is at least not returning successful responses for cases where disconnectedClientHelper.checkAndLogClientDisconnectedException returns true even though some internal exception occurred.
If you agree with my findings I can create a PR for this.
Comment From: rstoyanchev
For "connection reset by peer" or "broken pipe" I would expect the exception type is an instance of IOException
, and we could add that check to make it more specific.
However, it would be useful to find out more about your case. Is it exactly RuntimeException
with the phrase "connection reset by peer", is it raised by a JDK library class, or some other 3rd party library, and also what is the connection it is referring to?
I would also like to confirm that #34264 is not a regression. As far as I can tell it didn't change anything for your case.