Skip to content
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

Prevent errors in log when client closes the HTTP connection #2655

Merged
merged 1 commit into from
Dec 30, 2021

Conversation

cweitkamp
Copy link
Contributor

  • Prevent errors in log when client closes the HTTP connection

See openhab/openhab-distro#1188 (comment) and #2049.

@openhab/core-maintainers Please tell me your opinion.

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

@cweitkamp cweitkamp added the bug An unexpected problem or unintended behavior of the Core label Dec 29, 2021
// we catch this exception to avoid confusion errors in the log file, since this is not any error situation
// see https://github.com/openhab/openhab-distro/issues/1188
logger.debug("Failed writing HTTP response, since other side closed the connection");
// Returning null results in a Response.Status.NO_CONTENT response.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning null sounds just fine. In any case, the client won't receive what we return at all and null seems to best indicate that there's really nothing to return from our side.

// see https://github.com/openhab/openhab-distro/issues/1188
logger.debug("Failed writing HTTP response, since other side closed the connection");
// Returning null results in a Response.Status.NO_CONTENT response.
return null;
Copy link
Contributor Author

@cweitkamp cweitkamp Dec 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another solution:

Suggested change
return null;
return Response.noContent().build();

Or:

Suggested change
return null;
return Response.serverError().build();

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested it, but the code looks good and I would expect it to solve the issue.

// we catch this exception to avoid confusion errors in the log file, since this is not any error situation
// see https://github.com/openhab/openhab-distro/issues/1188
logger.debug("Failed writing HTTP response, since other side closed the connection");
// Returning null results in a Response.Status.NO_CONTENT response.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning null sounds just fine. In any case, the client won't receive what we return at all and null seems to best indicate that there's really nothing to return from our side.

@cweitkamp cweitkamp marked this pull request as ready for review December 30, 2021 10:22
@cweitkamp cweitkamp requested a review from a team as a code owner December 30, 2021 10:22
@kaikreuzer kaikreuzer merged commit adde43c into openhab:main Dec 30, 2021
@kaikreuzer kaikreuzer added this to the 3.3 milestone Dec 30, 2021
@cweitkamp cweitkamp deleted the bugfix-eofexception branch December 30, 2021 10:54
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
…#2655)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
GitOrigin-RevId: adde43c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core REST/SSE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants