Skip to content

Do not log an exception on connection closed by client (infamous Broken pipe error) - into 4.0.x branch #5583

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

Open
wants to merge 1 commit into
base: 4.0
Choose a base branch
from

Conversation

OndroMih
Copy link
Contributor

@OndroMih OndroMih commented May 6, 2025

Similar logic to ResourceHandlerImpl, which doesn't log an exception on connection closed. Exception is not logged because it's caused by the client, there's no way to recover and the warning message only pollutes logs. Now the code treats "connection closed" exception thrown by Grizzly.

Similar logic to ResourceHandlerImpl, which doesn't log an exception on connection closed. 
Exception is not logged because it's caused by the client, there's no way to recover and the warning message only pollutes logs.
Now also supports "connection closed" exception thrown by Grizzly.
@OndroMih OndroMih changed the title Do not log an exception on connection closed by client (infamous Broken pipe error) Do not log an exception on connection closed by client (infamous Broken pipe error) - into 4.0.x branch May 6, 2025
@OndroMih
Copy link
Contributor Author

OndroMih commented May 7, 2025

Hi @arjantijms , @BalusC , could you rerun the job https://github.com/eclipse-ee4j/mojarra/actions/runs/14871103597/job/41759489356 - it seems it's a flaky test as I only disabled logging a warning if client aborted the connection, no impact on main functionality.

@BalusC
Copy link
Contributor

BalusC commented May 25, 2025

I'm wondering if the new check can be more specific than that.

What's the actual exception class? Is it indeed IOException or is it actually something Grizzly specific such as org.glassfish.ConnectionClosedException? If so then please check exception class instead like as already implemented for Tomcat and Jetty.

What's the full exception message? Is it still "Connection is closed"? If so then please use equals instead of contains.

The flaky test has been fixed. Thanks for pointing out that! Please downmerge in order to re-run tests on your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants