-
Notifications
You must be signed in to change notification settings - Fork 38.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ExceptionHandlerExceptionResolver doesn't handle exceptions when the client closes its connection #30945
Comments
@nachtm thanks for the sample. Unfortunately, I couldn't reproduce this, all I got was |
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed. |
Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue. |
This is indeed a valid report, but hard to time since the network failure needs to happen exactly during response body writing in exception handling. There was a change in #26181 but it's currently in 6.1.x only, and also in the change we detect a disconnected client error but continue with handling of the original exception. We should flag it as resolved at that point, since failure to write to the response implies we did handle it, but didn't manage to write it out. As part of work for #32342 we are already making a wider backport of async request handling improvements, and those changes will cover this scenario too. So I am re-opening. |
@rstoyanchev should we create backport tickets for #26181 instead, both to 6.0.x and 5.3.x, and mark this one as a duplicate of it? It's currently a bit of an odd one out, marked for 5.3.x directly. |
Fair point. Also, the changes for #32342 didn't end up covering this in the end. In addition, I'd like to refine the solution for #26181, which suppressed logging but otherwise allowed continued handling of the original error. I think we should actually mark it as handled instead because we got as far as trying to write to the response, which means an |
Affects: 5.3.22
Description
@RestControllerAdvice methods for specific error classes can fail to apply if the client closes their connection during that time period.
Background
I have a @RestControllerAdvice class that handles the following scenarios (among many others):
We monitor our logs for ERROR-level logs, and potentially take action on them.
Problem
However, when a longer-running request gets abandoned on the client side, we're seeing the following behavior:
This is causing unnecessary noise in our logs, and more work for our engineers who need to triage these errors.
Request
I don't know a ton about the spring internals so don't necessarily want to make a guess as to how to fix this. Happy to throw out a couple ideas if you'd like, or leave it to the experts 😁
Reproduction
I've put up a small spring project reproducing the scenario we're seeing on our production servers: https://github.com/nachtm/spring-rest-controller-advice-error
The stacktrace is slightly different in production (we're hitting a
Broken pipe
instead ofConnection reset by peer
) but it seems close enough to get the point across.Stacktrace
Note the two WARN lines (fine) and then the one ERROR line (bad):
The text was updated successfully, but these errors were encountered: